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.
This commit makes the property setting in the `TestCase::set_up()` more flexible by combining an overloadable property `$propertyChanges` and a `foreach` loop to set the actual property values.
Concrete test classes can either overload the `$propertyChanges` property with their own version or can add to the default setup using the following pattern:
```php
protected function set_up()
{
$this->propertyChanges['additional_key'] = 'value';
// Add more properties...
parent::set_up();
}
```
The `TestCase::set_up()` was setting quite a number of properties in the `PHPMailer` class.
This makes testing more difficult for the following reasons:
1. The tests can no longer presume the properties in the `PHPMailer` class will have their default values
This means that tests are not "transparent" (clearly show what is being tested), nor isolated (only target what is specifically being tested).
2. Any changes to the values set in the `set_up()` method may have a ripple effect and create a need for individual test expectations to be adjusted.
3. As the `set_up()` is changing a number of the properties using methods in the `PHPMailer()` class and methods called during the `set_up()` are included in code coverage visualizations, code coverage cannot fully be trusted and it is more difficult to verify that each piece of code has tests covering that code path.
With this in mind, I'm proposing splitting the `TestCase` into three distinct abstract `TestCase`s:
* A basic `TestCase` containing the utility methods and a minimal `set_up()` and `tear_down()`.
* A `PreSendTestCase` for use with tests using the `preSend()` method which requires a few properties to be set.
* A `SendTestCase` for use with tests actually testing the sending of mail using the `send()` method, which needs yet more properties and uses the `testbootstrap.php` file to retrieve the values of those variables.
This commit executes the initial split. Follow-on commits will streamline this further.
Includes adjusting the `TestCase` being extended for select existing unit test classes.
A `]` is a special character in a regex and should be escaped when the literal is intended. While it was not problematic in this case, it is still recommended to escape it.
... 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 actually tested quite fully.
* Split the test into two tests with each a dedicated data provider
* Maintains the same test code and test cases.
* Makes it easier to add additional test cases in the future.
**Note**: the data set description may need some improvement, I've described them based on my best guess of what they were testing.
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`.
The postfix installation step fails regularly, resulting in failed CI builds which have to be restarted, while the failure is not due to anything in the PR.
This commit introduces a new action runner for the postfix install, which will automatically retry the install up to 3 times.
If it works as I expect it to, this should eliminate failed CI builds due to postfix installs erroring out.
Ref: https://github.com/marketplace/actions/retry-step
So far, this method did not have dedicated tests, though the `PHPMailerTest::testAttachmentNaming()` test covered this partially.
The test file this commit introduces, tests all aspects of the method as well as documents the current behaviour of the method.
Test cases largely inspired by the tests in the `PHPMailerTest::testAttachmentNaming()` 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 test from the original test file, just select parts of the test method.
