WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 7 weeks ago

#51734 new task (blessed)

Run core unit tests from src again, again

Reported by: johnbillion Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.1
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

Previously: #45863

When a developer first runs the test suite locally they are prompted to create a wp-tests-config.php file based on the wp-tests-config-sample.php file. This file defines the test directory for the core tests as build rather than src.

Running the core tests from build results in several problems:

  1. Running a build is slow. It copies all the files and builds, validates, and minifies all the CSS and JS. None of this should be necessary for PHP testing.
  2. A developer iterating on a patch in the source file has no way of knowing that their file is not actually being tested when running the tests, unless they run the build each time or start and run the file watcher. This is an easy step to forget.
  3. PHP errors display a stack trace from build instead src.
  4. Breakpoint debugging isn't fun as it also uses the stack trace from build instead of src.
  5. Any test which requires a build step is not a unit test and should be tested using other means, for example as a validation step during the build process.

Current Status

There are two tests that do not pass when running the test suite from the src directory.

The first is an error:

1) Tests_Dependencies_Scripts::test_no_source_mapping
UnexpectedValueException: RecursiveDirectoryIterator::__construct(wp/build/): failed to open dir: No such file or directory

wp/tests/phpunit/tests/dependencies/scripts.php:1406

The second is a test failure:

1) Tests_Dependencies_Scripts::test_wp_add_inline_script_before_after_concat_with_core_dependency
Failed asserting that two strings are identical.

[snip]

wp/tests/phpunit/includes/abstract-testcase.php:677
wp/tests/phpunit/tests/dependencies/scripts.php:755

Tasks

  1. For Tests_Dependencies_Scripts::test_no_source_mapping, find a more appropriate way to assert that built JS files do not contain a source map and remove this PHP unit test.
  2. For Tests_Dependencies_Scripts::test_wp_add_inline_script_before_after_concat_with_core_dependency (ticket #36392), fix the fact that this test expects .min.js in its output rather than using the return value of wp_scripts_get_suffix().
  3. Increase the reliability of tests which can mess up files if an assertion fails (tickets to follow).
  4. Change the default value of ABSPATH back to src in wp-config-sample.php.
  5. Investigate whether Travis and/or GitHub Actions should run their tests from src or build.

Change History (6)

This ticket was mentioned in PR #717 on WordPress/wordpress-develop by johnbillion.


2 months ago

  • Keywords has-patch added

#3 @johnbillion
2 months ago

  • Keywords has-patch removed

Update: There's actually many more errors and failures on a fresh checkout because the wp-includes/js directory gets tested but it's part of the build process.

https://github.com/WordPress/wordpress-develop/pull/717/checks?check_run_id=1376708592

#4 @peterwilsoncc
2 months ago

Tests_Embed_Template::test_js_no_ampersands_in_compiled() is a special case in which a built file is tested against in the unit tests, the /build/ directory is hard-coded and it fails silently if the file doesn't exist.

Please be sure to allow for this in any refactoring.

#5 @johnbillion
2 months ago

In 49601:

Build/Test Tools: Improve the reliability of the test that ensures correct dependency order when inline scripts are used.

This test already ensures WP_Scripts->do_concat is true, therefore it has no dependency on SCRIPT_DEBUG being false. This means the test can run in an environment where the .min suffix is not used.

This change allows for the test to pass in this situation.

See #36392, #51734, #51344

#6 @iandunn
7 weeks ago

Related: r48847 updates phpunit.xml.dist to track code coverage against the build/ folder.

Note: See TracTickets for help on using tickets.