This commit moves the "set a property to null and then back" test to a separate test method.
Note: based on the code in the method, this test doesn't really add any value, but I also see no reason to remove it.
... to test that passing an invalid email address in combination with an instance of the `PHPMailer` class which was instantiated with `$exceptions = true` results in an exception.
Includes reworking the `testSetFromFail()` method to a data provider and letting both the `testSetFromFail()` and the new `testInvalidAddressException()` method use the same data provider.
Make the failure test more comprehensive by verifying that when the method fails, the values for the `From`, `FromName` and `Sender` properties, _really_ haven't changed.
Based on the code in the method, any existing, previously set `Sender` should not be overruled, even when the `$auto` parameter is set to `true`.
This method tests that specific situation.
* Merges the two "success" tests from the `testAddressing()` and the `testAddressing2()` methods into one `testSetFromSuccess()` method.
* Adds additional assertions to more comprehensively verify that the method did what was expected, i.e. set the `From`, `FromName` and `Sender` properties.
* Maintains the same test cases.
* Makes it easier to add additional test cases in the future.
So far, these methods were only tested in the most perfunctory manner.
The additional tests this commit introduces, test all aspects of the methods as well as documents the current behaviour of the methods.
Take note of the "text merging"/readability issues for the SMTP error messages. There may be room for improvement there.
... by a much simpler test which effectively tests the same thing, i.e.:
* No errors to start with.
* Trigger an error.
* Verify that `PHPMailer::isError()` returns `true`.
* Verify that the error message is as expected.
What with the previous commit adding a requirement for the `Mbstring` extension to the existing tests, it becomes clear that the "Mbstring extension not available" code path was not covered by the tests.
This commit fixes that by:
* Adding two new test methods which explicitly expect the Mbstring extension to **not** be available.
* Changing the data provider "expected(Imap)" keys.
The `expected` key in the array will now be an array of arrays.
A `default` key can be used for when the output across configurations will be the same.
Any differences across configurations can be provided in separate sub-keys of the `expected` array, using the `native+mbstring`, `imap+mbstring`, `native--mbstring` and/or `imap--mbstring` keys, which match the four test methods which are now in place.
Additionally, an `native` and/or `imap` key can be used for setting the output expectations for the two native implementation or the two IMAP implementation tests, if the Mbstring extension makes no difference.
Follow up to #2266
Bug fix
In both "arms" (imap vs native implementation) of the `PHPMailer::parseAddresses()` method, the `mb_decode_mimeheader()` function is used to decoded a (utf-8) encoded name.
In the IMAP "arm", a check was in place for the Mbstring extension being available before using it. This check was missing from the "native implementation" "arm".
Existing Tests
This also means that both currently existing tests have a requirement for the MbString extension being available. This was previously not made explicit in the tests.
Fixed now.
... which should be handled correctly based on the code in the method under test.
With these additional test cases, the method now has 100% code coverage and is fully tested.
Using a regex assertion with arbitrary input data which is not regex escaped, makes this test suspect.
From the looks of it, the test _should_ be testing that the output is the _same_, so let's use that assertion.
This commit:
* Adds a new `testFailToAttach()` test method to test the behaviour of the `PHPMailer::AddStringEmbeddedImageTest()` method when the `PHPMailer` class has been instantiated with `$exceptions` disabled.
* This new test method uses the same data provider - introduced in the previous commit - as the `testFailToAttachException()` method.
This commit:
* Renames the `testStringEmbeddedImageEncodingException()` test to `testFailToAttachException()`.
* Reworks the test to use a data provider.
* Adds testing of the exception message to the `testFailToAttachException()` method.
This commit:
* Improves the test name and the description in the docblock.
* Replace a redundant condition and "forced" failure assertion with an assertion actually testing the result of the method call.
* Removes the redundant `return` - if an assertion fails, the rest of the code within the test method will not be executed anyway.
* Minor inline comment tweaks.
The actual "attaching" of the string attachment happens within `createBody()` which is called from `preSend()`, so this test doesn't actually need to call `send()`.
So far, this method did not have dedicated tests.
The test file this commit introduces, tests nearly all aspects of the method as well as documents the current behaviour of the method.
There is one particular test case missing. This is annotated in the class docblock.
While this method is quite simple, testing it separately means that the tests for methods _using_ this method don't have to _also_ test the functioning of this method, which means they can be more focussed on their own logic.
So far, this method did not have dedicated tests.
The test file this commit introduces, tests all aspects of the method as well as documents the current behaviour of the method.
While this method is quite simple, testing it separately means that the tests for methods _using_ this method don't have to _also_ test the functioning of this method, which means they can be more focussed on their own logic.
This commit:
* Adds a new `testFailToAttach()` test method to test the behaviour of the `PHPMailer::addStringAttachment()` method when the `PHPMailer` class has been instantiated with `$exceptions` disabled.
* This new test method uses the same data provider - introduced in the previous commit - as the `testFailToAttachException()` method.
This commit:
* Renames the `testStringAttachmentEncodingException()` test to `testFailToAttachException()`.
* Reworks the test to use a data provider.
* Adds testing of the exception message to the `testFailToAttachException()` method.
The actual "attaching" of the string attachment happens within `createBody()` which is called from `preSend()`, so this test doesn't actually need to call `send()`.
This commit:
* Adds an `exceptionMessage` index to the `dataFailToAttach()` data provider.
* Renames the `testEmbeddedImageEncodingException()` method to `testFailToAttachException()`.
* Sets the `testFailToAttachException()` method up to use the `dataFailToAttach()` data provider.
* Adds testing of the exception message to the `testFailToAttachException()` method.
With this change, the "fail to attach" test cases are now fully tested for both a `PHPMailer` instance without exceptions enabled, as well as for an instance _with_ exceptions enabled.
The test code remaining in the `testEmbeddedImage()` constitutes one test.
This commit:
* Renames the test method and improves the description in the docblock.
* Removes the redundant call to `PHPMailer::clearAttachments()`.
This call was previously needed as multiple situations were being tested in one test method.
Now each test case has its own test method, the call to `PHPMailer::clearAttachments()` is no longer needed as each test method will receive a fresh, clean instance of the `PHPMailer` class.
* Replace a redundant condition and "forced" failure assertion with an assertion actually testing the result of the method call.
* Removes the redundant `return` - if an assertion fails, the rest of the code within the test method will not be executed anyway.
The test case when a file was attached without explicitly adding a filename wasn't actually being tested at all as no assertion was used.
This commit:
* Moves that particular test case to a separate test method.
* Adds relevant assertions to actually test the test case.
The "failure" case when a non-existent file was being passed, wasn't actually being tested at all as no assertion was used.
This commit:
* Moves the particular failure test case to a separate test method with a data provider (to allow for more failure test cases to be added).
* Uses an assertion on the call to `addEmbeddedImage()` to actually test that the method return a failure state.
* Verifies that no attachment for an inline image was added by adding a second assertion with a call to `PHPMailer::inlineImageExists()`.
The actual "attaching" of the images happens within `createBody()` which is called from `preSend()`, so this test doesn't actually need to call `send()`.
* Maintains (largely) the same test cases.
* Prevent one failing assertion hiding a potential second failure.
* Makes it easier to add additional test cases in the future.
Note:
This removes the intermittent calls to `clearCustomHeaders()` from this test. This is now tested via a separate method and as each test case will receive a fresh instance of the `PHPMailer` class, there is no need to clear the set custom headers between tests.
The line length adjustments are executed within `createBody()` which is called from `preSend()`, so this test doesn't actually need to call `send()`.
I also wonder if this test can be further improved by having more targeted tests for `PHPMailer::hasLineLongerThanMax()` and whether these more extensive tests can be included in a test class which is more targeted at the `createBody()` and/or `preSend()` methods, but that is for later.
Previously, the test didn't actually test whether the wordwrapping had been applied, just that the message was (pre)send successfully.
Changing the assertions to actually test that the wordwrapping has been correctly applied.
For this test, there is no need to actually try to _send_ the message, we just need to make sure that `setWordWrap()` is triggered, which can be done by calling `preSend()` instead of `send()`.
This new test method covers a range of cases where the `PHPMailer::punyencodeAddress()` method should (and does) return the original input value unchanged.
This test does not require the `mbstring` extension or `idn_to_ascii()` function to be available, which is why it has been set up as a separate test with a separate data provider.
The "fakefunctions" are all nice and dandy to get past the `idnSupported()` check, but if either of these functions is not _really_ available and therefore doesn't behave as expected, the test would still fail as the expected output of the `PHPMailer::punyencodeAddress()` function would not match.
In other words, this test should not use the `fakefunctions`, but should have a hard requirement for the `mbstring` extension (for the `mb_check_encoding()` and the `mb_convert_encoding()` function calls) and a check for the `idn_to_ascii()` function.
Follow up on 2389 and 2412
Now the base `TestCase` has been simplified and only presets the bare minimum of properties in the `PHPMailer` class, the `Utf8CharBoundaryTest` can actually use it.
The `PHPMailerTest` class contained a `testBootstrap()` method to verify that the `testbootstrap.php` file exists as the first test in the class.
As the order in which tests are run is not predefined, this is not reliable.
Additionally, the check for the `testbootstrap.php` file is checking a pre-requisite for tests using the `PHPMailer::send()` method, so it would be better to verify via a condition in the `set_up()`.
Now, using such a condition, there is choice: the test can either be marked as "skipped" when the `testbootstrap.php` file can not be found, or be marked as an "error".
As the tests _should_ run, skipping them would hide an error in the dev-user test setup, so showing these tests as errors seems more appropriate.
To that end, the `testBootstrap()` method has been removed and a check for the `testbootstrap.php` file has been added to the `SendTestCase::set_up()` which will now throw an appropriate `Exception` when the file is not found.
Oops... also removes a stray `parent::set_up()` at the start of the local `set_up()` which should have been removed in 218fd13c88
Note: as the `PHPMailer::generateID()` method is `protected`, a round-about way of testing this is needed, but this test does verify the functioning of the method.
Take note of the notes in the test docblock - the GH Actions scripts running CI, should make sure that each of these scenarios is encountered/tested.
This will be addressed in a separate PR at the end of this round of test changes.
Minor test tweaks:
* Add `@coversNothing` tag.
* Rename the first test (add a number).
* Use `@link` instead of `@see` for links.
* Minor comment punctuation.
The `TestCase::checkChanges()` method is a way of exposing what properties in the `PHPMailer` class have a changed value compared to their default value in a particular test situation. The method is used for debugging tests.
As things were, the `TestCase::checkChanges()` method would check against a limited set of hard-coded values to determine whether the default value of a property has been updated.
This is unstable as:
1. Default values may change in the `PHPMailer` class and the values within this method would need to be updated to match, which is easily forgotten.
2. New properties may be introduced in the `PHPMailer` class and be relevant to this debug changelog.
Again, it would require manually adding these new properties to this method to start tracking them.
3. Property values may be changed in the `set_up()` method, which would be a "known change" for a certain test.
In part such "expected" changes were taken into account in this method based on the previously hard-coded setting changes in `set_up()`.
With the logic for the property setting from the `set_up()` method now being more flexible, the pre-setting of properties having been reduced to the bare minimum, but also allowing individual test clases to set their own additional changes, keeping track of what is a "known" change by checking against hard-coded values is no longer stable.
With this in mind, I propose to make the `TestCase::checkChanges()` method dynamic.
To that end, this commit:
* Retrieves the default values of all properties of the `PHPMailer` class via the PHP native `get_class_vars()` function.
* Will automatically check for changes in *all* properties, with only a limited set of _exclusions_, effectively changing the changelog from an "inclusion list" to an "exclusion list".
A select list of properties is excluded from being listed in the changelog via the `TestCase::$changelogExclude` property.
See the inline documentation in the property for the reasoning behind excluding certain properties from the changelog.
* The value of static properties will always be compared to their default value as registered in the `TestCase::$PHPMailerStaticProps` method and will be listed when different.
_Note: as documented, this list has to be hard-coded due to Reflection (as well as `get_class_vars()`) not handling default values for static properties correctly._
* The value of non-static properties will be compared to both the known changes made in the `set_up()` method and if the property was not changed in `set_up()`, to their default value. The property will be listed in the changelog when the value is different from the "expected" value, i.e. not a known change from `set_up()` and not the default value.
In addition to this, the representation of the properties will now be created via `var_export()`, which allows for listing `null` and boolean values as well.
The `SendTestCase` gets the values of the properties to be set from the `testbootstrap.php` file.
This introduces a `private` property to map the field names used in `$_REQUEST` to the properties in the `PHPMailer` class and adds logic to the overloaded `set_up()` method to fill the `$propertyChanges` TestCase property. The actual setting of the properties in the `PHPMailer` instance is deferred to the underlying `TestCase` parent class.
Includes adding support for presetting the `bcc` value for feature completeness.
Overloading and/or adding to the `$propertyChanges` array from concrete test cases is, of course, supported, so if individual tests need additional presetting of properties, the same logic as mentioned in the previous commit can be used.
After some investigation, it turns out that barely any of these properties are actually needed for the `PHPMailer::preSend()` method to succeed.
This commit removes all presetting of properties for the PHPMailer instance created by the `PreSendTestCase`, save for the bare minimum.
Overloading and/or adding to the `$propertyChanges` array from concrete test cases is, of course, supported, so if individual tests need additional presetting of properties, the same logic as mentioned in the previous commit can be used.
