Make WordPress Core

Opened 4 weeks ago

Closed 3 weeks ago

#65894 closed enhancement (fixed)

Remove the unused wp-mail-real-test.php from the PHPUnit test suite

Reported by: lancewillett Owned by: SergeyBiryukov
Priority: normal Milestone: 7.2
Component: Build/Test Tools Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description (last modified by SergeyBiryukov)

tests/phpunit/wp-mail-real-test.php is a standalone CLI script that installs WordPress and calls wp_mail() twice against real Gmail addresses. It is not a PHPUnit test, nothing runs it, and it has been unable to run since 2012.

It is not part of the test suite

The default testsuite in phpunit.xml.dist globs only tests/phpunit/tests, so this file has never been collected by a PHPUnit run. It has no test class and no assertions on delivery. It prints Test emails sent! and leaves a human to check the inbox.

Nothing in src/ or tests/ references it. No Trac ticket mentions it by name.

It cannot run

Three of its require paths point at files that do not exist in this repository:

  • DIR_TESTROOT . '/wp-testlib/getopt.php' (line 13)
  • DIR_TESTROOT . '/wp-testlib/utils.php' (line 34)
  • DIR_TESTROOT . '/wp-config.php' (line 37)

The last two are unconditional, so the script fatals on any invocation. wp-testlib/ has never existed anywhere in develop.svn history.

It was never meant to be automated

The pre-2013 history is in the old unit-tests repository, still reachable at unit-tests.svn.wordpress.org (currently r1337). All revision numbers below are from that repository, not core.

Two commits on 2011-05-23, both by westi, 38 minutes apart:

  • [359/tests] (03:33) added test_wp_mail_rfc2822_addresses() and test_wp_mail_multiple_rfc2822_to_addresses() to wp-testcase/test_includes_pluggable.php. These assert against the mock mailer, under #17305.
  • [360/tests] (04:11) added this script, copied from wp-test.php@358: "Add a simple test script for sending real emails with wp_mail to check that it actually works too."

So the script was deliberately a manual companion to the automated tests, not a test itself. It uses the same RFC2822 Testing subject and My RFC822 Test Message body as [359/tests]. It was the by-hand version of the same scenario.

Its only other revisions in that repository were cosmetic: [r368/tests] set svn:eol-style, [407/tests] was "Pinking shears" whitespace, [751/tests] moved everything into trunk/.

It broke at [875/tests]

[875/tests] (2012-07-03, nacin) restructured the suite for PHPUnit: renamed wp-testlib to includes, deleted includes/getopt.php and includes/base.php, renamed wp-config-sample.php to wp-tests-config-sample.php, and deleted wptest.sh.

wp-mail-real-test.php was not updated. All three of its requires became invalid in that one revision. utils.php survived the rename and lives at tests/phpunit/includes/utils.php today, but the script still points at the old path.

The file was carried into core in that broken state 13 months later, with the 2013 initial import of the unit-tests repository into develop.svn.

The cost

Ten commits in core since the import, every one of them cosmetic:

Date Change
2013-08-29 Moved to tests/phpunit/
2015-10-21 Dropped a $_SERVER['SERVER_NAME'] line (#34394)
2017-11-30 WPCS reformat, 42 of 85 lines touched (#41057)
2019-01-11 Added a phpcs:ignore for PHPUnit_MAIN_METHOD
2019-07-08 Further PHPCS fixes in /tests
2020-01-29 Comment style
2020-02-06 dirname( __FILE__ ) to __DIR__
2022-10-21 US spelling
2023-09-07 Removed a trailing blank line

None changed behavior. phpcs.xml.dist scans the tree via <file>.</file>, so each standards sweep picked the file up automatically and someone reviewed the result. Core has spent 13 years applying standards and review time to a script that has fatalled on every invocation for 14.

Coverage is not lost

tests/phpunit/tests/pluggable/wpMail.php has 30 tests, including the direct descendant of [359/tests] at line 96:

public function test_wp_mail_rfc2822_addresses() {

Same subject, same message body, same To/From/CC/BCC shape, but asserted through MockPHPMailer (loaded by tests/phpunit/includes/bootstrap.php) instead of sent to Gmail. Deleting the script removes nothing that is checked anywhere else.

Proposed

Delete tests/phpunit/wp-mail-real-test.php.

It also hardcodes wp.mail.testing@gmail.com and three + aliases, which is a live third-party address sitting in the repository for a script that cannot run.

If a real-delivery smoke test is wanted later, it is better written against the current bootstrap than revived from this one.

Change History (4)

#1 @lancewillett
4 weeks ago

Related: #65888 removes tests/phpunit/includes/wp-profiler.php on the same grounds.

This ticket was mentioned in PR #13094 on WordPress/wordpress-develop by @lancewillett.


4 weeks ago
#2

  • Keywords has-unit-tests added

## What this changes

Removes the unused tests/phpunit/wp-mail-real-test.php file.

The script is not a PHPUnit test and is never collected by a test run. It has no callers, and three of its require paths point at files that do not exist, so it fatals on any invocation.

Its scenario is already covered with assertions by test_wp_mail_rfc2822_addresses() in tests/phpunit/tests/pluggable/wpMail.php, which exercises the same To/From/CC/BCC headers through MockPHPMailer.

Removing it prevents future standards sweeps and review time from landing on a script that cannot run. If a real-delivery smoke test is wanted later, it can be built against the current bootstrap.

## Testing

  • git diff --check passed.
  • Confirmed no references to wp-mail-real-test remain anywhere in the repository.
  • Confirmed the default testsuite in phpunit.xml.dist globs only tests/phpunit/tests, so the file was never collected.
  • Confirmed tests/phpunit/wp-testlib/ and tests/phpunit/wp-config.php, which the script requires, do not exist.
  • Ran the wp_mail() suite on this branch: phpunit --filter 'Tests_Pluggable_wpMail' gives OK (32 tests, 87 assertions), so the RFC2822 address handling the script exercised is still covered.

No runtime tests were added because this removes an unreachable script.

#3 @SergeyBiryukov
3 weeks ago

  • Description modified (diff)
  • Milestone Awaiting Review7.2

#4 @SergeyBiryukov
3 weeks ago

  • Owner set to SergeyBiryukov
  • Resolutionfixed
  • Status newclosed

In 63341:

Build/Test Tools: Remove unused wp_mail() real-send script.

The tests/phpunit/wp-mail-real-test.php script is not a PHPUnit test and is never collected by a test run. It has no callers, and three of its require paths point at files that do not exist, so it fatals on any invocation.

Its scenario is already covered with assertions by test_wp_mail_rfc2822_addresses() in tests/phpunit/tests/pluggable/wpMail.php, which exercises the same To/From/CC/BCC headers through MockPHPMailer.

Removing it prevents future standards sweeps and review time from landing on a script that cannot run. If a real-delivery smoke test is wanted later, it can be built against the current bootstrap.

Developed in https://github.com/WordPress/wordpress-develop/pull/13094.

Follow-up to [360/tests].

Props lancewillett.
Fixes #65894.

Note: See TracTickets for help on using tickets.