[wp-trac] [WordPress Trac] #44233: Add missing unit tests for exporting personal data by username or email address

[WordPress Trac]

#44233: Add missing unit tests for exporting personal data by username or email
address
-------------------------+------------------------
 Reporter:  desrosj      |       Owner:  desrosj
     Type:  enhancement  |      Status:  reviewing
 Priority:  low          |   Milestone:  4.9.9
Component:  Privacy      |     Version:  4.9.6
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |     Focuses:
-------------------------+------------------------

Comment (by desrosj):

 Thanks, @iandunn! See my responses below which are reflected in
 [attachment:"44233.7.diff"] .

 ==== `Tests_Privacy_WpPrivacyProcessPersonalDataExportPage`

 > How is `$request_email` different from `$requester_email`?

 `$request_email` appears to left over from a patch refactor. I removed it
 because it was not used anywhere (`$requester_email` was used instead in
 all test methods).

 > Why is this (`wp_mail_from` hook) removed here instead of in
 `test_function_should_send_error_on_last_page_of_last_exporter_when_mail_delivery_fails()`?

 I removed all filter and action related removing in the `tearDown()`
 method because `parent::tearDown()` already handles these.

 Also in this class:

 - Removed `self::$send_as_email`. This is only ever `true` or `false`. I
 created a data provider for methods that should test when an email is and
 is not sent.


 ==== `Tests_Privacy_WpPrivacyGeneratePersonalDataExportFile`

 > What problem is caused by a plugin intentionally changing the path to
 the folder?

 The `tearDown()` method will delete the export folder after each test
 method. I think @birgire had added this, but I think the intent was to
 prevent a non-default export folder from being removed when running the
 tests on an install where a plugin was filtering the directory. I removed
 these checks in my patch. If @birgire can point to a reason this should
 stay, we can add the checks back.

 > Is it unsafe to assume that `$exports_dir` is a folder? What would cause
 it to be a file?

 The exports directory will be a file after the
 `test_detect_cannot_create_folder()` test method which verifies an error
 is returned when the export directory is a file. I added an inline comment
 above that conditional for context. It also ''could'' be a file if an
 incorrect value is returned to the `wp_privacy_exports_dir` filter.

 Also in the patch:

 - Removed `_function_` from all methods that had
 `test_function_does_something` for more precise naming.
 - Created a DRY `_setup_expected_failure()` helper method to set up a test
 method to expect an exception.
 - Removed `@since` tags on test methods in favor of `@ticket` tags.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/44233#comment:26>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform