Running against stable/lowest dependencies is relevant when a package has runtime (non-dev) dependencies.
However, PHPMailer does not have runtime dependencies.
In other words, the `dependency-version` matrix key is redundant and unused, so we may as well remove it.
As this test does not actually need an instantiated PHPMailer object, this class extends the `Yoast\PHPUnitPolyfills\TestCases\TestCase` instead of the `PHPMailer\Test\TestCase`.
Note: this doesn't move the complete test from the original test file, just select parts of the test method.
As this test does not actually need an instantiated PHPMailer object, this class extends the `Yoast\PHPUnitPolyfills\TestCases\TestCase` instead of the `PHPMailer\Test\TestCase`.
Note: this doesn't move the complete tests from the original test file, just select parts of two test methods.
The `PHPMailer::preSend()` method may alter the value of the _`protect static`_ `PHPMailer::$LE` property.
a9978d2079/src/PHPMailer.php (L1494-L1504)
This means that tests which rely on the value of the property being the default, may start to fail unexpectedly depending on the _order_ in which tests are being run.
I.e. if the test relying on the default value is run first, all will be fine, however, if that same test is run _after_ a test which calls `preSend()` on a non-Windows box with PHP < 8.0 and with `$this->Mailer` set to `mail`, the test may fail unexpectedly.
This commit fixes that by expanding the previously introduced "resetting of static properties" logic.
This commit:
* Expands the list of static properties in the `TestCase` class to include the `PHPMailer::$LE` property.
* Introduces two new methods to the `TestCase` class:
- `resetStaticProperties()` to reset all static properties listed in the `TestCase::$PHPMailerStaticProps` property to their default values.
- `updateStaticProperty()` to change the value of one individual static property in a class.
* The `updateStaticProperty()` method was made `public static` to allow for tests which do not need an instantiated PHPMailer object and thus extend the `Yoast\PHPUnitPolyfills\TestCases\TestCase` directly to be able to use the method as well.
If any such tests would use the method, that particular test will be responsible for resetting the value back to the default after the test.
* The resetting of the static properties has also been moved from the `set_up()` to the `tear_down()` method.
As mentioned above, these static properties may be used by both tests extending this `TestCase` as well as test extending the `Yoast\PHPUnitPolyfills\TestCases\TestCase`. With that in mind, resetting _after_ each test will be more stable, as long as tests which extend the `Yoast\PHPUnitPolyfills\TestCases\TestCase` and would change static properties clean up after themselves.
**Note**: I'm not including the `PHPMailer::$IcalMethods` property in this change as AFAICS, this property is not being changed anywhere in the package code and as the property is `protected`, it cannot easily be changed from within a test either (and if it would be changed by a test, that test would be responsible for resetting it).
Also note: due to limitations in the `Reflection` extension, a hard-coded list of the default values of the static properties will need to be maintained in the `TestCase`. This cannot be helped as the `Reflection` extension does not have a way to accurately retrieve the default value of static properties in a PHP cross-version compatible manner.
The `PHPMailer::__construct()` method has an optional `$exceptions` property to throw exceptions on errors.
Up to now, the `PHPMailer` instance created by the `set_up()` would not pass any parameters, effectively instantiating `PHPMailer` without turning on exceptions.
In a test situation it may be useful for tests to test the behaviour of a method _with_ and _without_ the exceptions option and to test that certain exceptions are thrown and throw the correct message.
With that in mind, I'm introducing a `USE_EXCEPTIONS` class constant to the `TestCase` which can be overloaded in individual test classes and will be used by the `set_up()` method to determine whether it will be instantiated with exceptions or not.
Includes introducing an overload of the class constant in one of the test class for which it would seem appropriate at this time.
Note: this does mean that the `testDKIMSigningMail()` test will show as errored instead of failed if no SMTP connection could be made, but that IMO is the more correct status anyhow.
This allows for using the `setAddress()` method in a more consistent manner (where appropriate).
Includes introducing the use of the `setAddress()` function in a few select places.
Note: I do wonder whether this method should ever be used outside of `set_up()` and `tear_down()`, but that is for further discussion and outside the scope of this commit.
While this class does not (currently) _change_ the `PHPMailer::$validator` property, it does rely on the default value being the expected default.
By resetting the value of the property before and after the class, this is safeguarded for the current tests.
Public static properties changed by individual tests were not being reset to their default value prior to the next test being run.
This could influence the test results of subsequent tests as static properties are not automatically reset when a new instance of a class is instantiated. See: https://3v4l.org/8s1RB
Luckily this didn't aversely affect the tests so far, but should be safeguarded for the future.
There are only three static properties in the `PHPMailer` class, with only one of these being `public`.
This commit introduces a code snippet which will reset any static properties as listed in the `$PHPMailerStaticProps` property of the `TestCase` to the default value, as also set in the `$PHPMailerStaticProps` property, at the start of the `set_up()` which is run before each test using the `TestCase`.
For now, this is only relevant for the `ValidateAddressCustomValidatorTest` test and only when the `testSetDefaultValidatorToCustom()` test would fail before resetting the property. All the same, having the reset in place by default will ensure that future tests which change this property won't introduce side-effects to other tests.
... in part to document the behaviour of the function, in part to actually test it.
By the looks of it more tests are still needed though to raise the code coverage of this function to 100%.
I'd also recommend adding tests with different charset settings as the current tests are all based on the default charset (`CHARSET_ISO88591`).
Also note: the "empty string" test may need looking at.... is this a bug ?
* Maintains the existing test cases.
* Prevent one failing assertion hiding a potential second failure.
* Makes it easier to add additional test cases in the future.
As this test is marked _incomplete_, no further review of the test has been done and no `@covers` tag has been added.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move utf8CharBoundary test to own file
* Utf8CharBoundaryTest: reorganize to use data providers
This:
* Renames the test method and moves the description to the docblock.
* Decouples the test from the PHPMailer `TestCase` as it doesn't need the complete `set_up()` and `tear_down()`.
* Moves the test cases to a data provider.
* Adds a `@covers` tag.
* Utf8CharBoundaryTest: add `@todo` reminder
This really could do with some more test cases to properly cover all paths and branching in the method.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move address splitting tests to own file
As this test does not actually need an instantiated PHPMailer object, this class extends the `Yoast\PHPUnitPolyfills\TestCases\TestCase` instead of the `PHPMailer\Test\TestCase`.
* ParseAddressesTest: reorganize the testAddressSplitting() test
This commit:
* Adds two new test methods, one for testing address splitting using the PHPMailer native implementation, one for testing address splitting using the IMAP implementation.
Both these methods use the same data provider.
* Adds a PHPUnit `@requires` tag for the IMAP extension to the IMAP version of the test.
* Adds a type check `assertIsArray()` before doing the content checks on the method return value.
* Changes the tests to do a more detailed `assertSame()` test instead of the previous `assertCount()` and `assert[Not]Empty()` checks.
* Sets up the data provider to have the detailed output arrays for comparison.
The actual test cases in the rewritten test are the same as were previously being tested.
* ParseAddressesTest: replace remaining test by dataprovider entry
The `testImapParsedAddressList_parseAddress_returnsAddressArray()` and the `testImapParsedAddressList_parseAddress_returnsAddressArray_usingImap()` tests were largely duplicates of each other, with the only real difference being the value of the second parameter passed to the `PHPMailer::parseAddresses()` method (`$useimap`).
This commit:
* Removes these two tests in favour of adding the test case to the data provider added in the previous commit.
In effect, the same test case is still being tested just as thoroughly now, just with less (test) code.
* ParseAddressesTest: add two more (invalid) test cases to `dataAddressSplitting()`
* PHPMailer::parseAddresses(): bug fix
If invalid email addresses are passed to any of the IMAP functions - in this case to `imap_rfc822_parse_adrlist()` - IMAP may generate error notices or warnings.
If nothing is done with these, they will be displayed at PHP shutdown.
The notice will look something like this, for instance:
```
Notice: Unknown: Missing or invalid host name after @ (errflg=3) in Unknown on line 0
```
This bug was exposed by the newly added unit tests. You should be able to see the issue if you run the tests over the previous commit.
As PHPMailer is not interested in invalid addresses, we can just ignore these notices and errors, but we *do* have to _clear_ them to prevent the notices from being thrown.
Refs:
* https://www.php.net/manual/en/function.imap-rfc822-parse-adrlist.php
* https://www.php.net/manual/en/function.imap-errors.php
* https://github.com/ddeboer/imap/issues/308
* https://stackoverflow.com/questions/3378469/how-to-get-rid-of-error-messages-with-phps-imap-fetchstructure
* ParseAddressesTest: reorder the test cases in `dataAddressSplitting()`
... to a slightly more sane order for humans to follow and see at a glance what is being tested..
* ParseAddressesTest: add `@covers` tags
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move message ID test to own file
* GetLastMessageIDTest: use the correct parameter order
For PHPUnit assertions which expect an `$expected` and a `$result` parameter, the parameter order is always `( $expected, $result, ...).
While it may not seem important to use the correct parameter order for assertions doing a straight comparison, in actual fact, it is.
The PHPUnit output when the assertions fail expects this order and the failure message will be reversed if the parameters are passed in reversed order which leads to confusion and makes it more difficult to debug a failing test.
* GetLastMessageIDTest: split into separate tests
* GetLastMessageIDTest: reorganize to use data providers
While initially still only addressing the original test cases, using a data provider here will allow for adding additional test cases more easily.
* GetLastMessageIDTest: add additional tests
Adding additional tests based on the code this is supposed to be testing.
**Note**: I've put the test cases in the "valid"/"invalid" data providers based on the current reality.
Some of the test cases I've added to the "valid" data provider _might_ actually be **_invalid_**. If that's the case, the regex used on line 2554 needs adjusting to account for them (after which the test cases can be moved to the "invalid" data provider).
* GetLastMessageIDTest: add `@covers` tags
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move line length detected tests to own file
* HasLineLongerThanMaxTest: various test tweaks
Minor test tweaks:
* Add `@covers` tag.
* Inline comment punctuation.
* Minor code readability tweaks.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move ICal tests to own file
* ICalTest: reorganize to use data providers
These three tests were 99% duplicate code. Using a data provider removes the duplication, while the actual tests being run are still 100% the same, including same test count and assertion count.
* ICalTest: add additional tests
... to ensure all valid methods are checked.
Adding these extra tests is a breeze now with the data provider setup.
* ICalTest: various test tweaks
Minor test tweaks:
* Add `@covers` tag. **_<- is this correct ?_**
* Minor comment punctuation.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move OAuth test to own file
This test uses the `Yoast\PHPUnitPolyfills\TestCases\TestCase` instead of the `PHPMailer\Test\TestCase` base class as it doesn't need the `set_up()` or `tear_down()` methods from the PHPMailer base test class.
* OAuthTest: various test tweaks
Minor test tweaks:
* Add `@covers` tags.
* Add "failure" messages to assertions.
* Minor comment punctuation.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move POP before SMTP tests to own file
This also removed the `$pids` property and handling from the PHPMailer base `TestCase` and moves this to the POP test class.
As this test now does not actually need an instantiated PHPMailer object, this class extends the `Yoast\PHPUnitPolyfills\TestCases\TestCase` instead of the `PHPMailer\Test\TestCase`.
* PopBeforeSmtpTest: move `@group` tags
... to the class level and remove them from the individual test functions.
* PopBeforeSmtpTest: various test tweaks
Minor test tweaks:
* Add `@covers` tag at class level.
* Inline comment punctuation.
* Minor code readability tweaks.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move DKIM related tests to own file
* DKIMTest: stabilize removal of private key file
When an assertion in a test fails, the rest of the code within the test method is no longer executed.
This means that for a test which - like three out of the five DKIM test - creates a file, the test "clean up" via `unlink()` is also no longer executed.
It also means that one test may - unintentionally - interfere with the results of another test due to the file still existing, while it is expected to have been removed.
Aside from that, the `$privatekeyfile` variable used in the various test is the same everywhere, but not consistently used in the tests, as the `'dkim_private.pem'` is hard-coded in multiple places, which decreases maintainability.
This commit fixes both these issues by:
* Declaring a class constant with the target private key file name for use in the test method.
* Implements use of this constant throughout the tests.
* Removes the `unlink()` call from the individual tests, in favour of executing it via the test `tear_down()`, which should still be executed, even when a test has failed.
Note: as the file also contains two tests which do not create the private key file, but for which the `tear_down()` would also be executed, the `unlink()` call is wrapped in a `file_exists()`.
* DKIMTest: move `@group` tags
... to the class level and remove them from the individual test functions.
* DKIMTest: add missing `@requires` tags
Three out of the five tests actually require the `openssl` extension. Only one was so marked.
* DKIMTest: various test tweaks
Minor test tweaks:
* Rename a test to a more specific name to allow for easier test filtering via PHPUnit.
* Add `@covers` tags (Needs review!)
* The `@see` tag is indented for code elements. For external links, the `@link` tag should be used.
* Inline comment punctuation.
* Minor code readability tweaks.
* DKIMTest: add two additional tests
... to cover the "open SSL" not available case.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
* Tests/reorganize: move email transport tests to own file
* MailTransportTest: various test tweaks
Minor test tweaks:
* Add `@covers` tags (Needs review! - especially the `testMailSend()` method seems to do more than it should)
* Check if test skipping is necessary at the start of a test method.
* Add "failure message" for each assertion in tests with multiple assertions.
* Tidy up inline comments.
Co-authored-by: jrfnl <jrfnl@users.noreply.github.com>
jrfnl
Marcus Bointon
Juliette