Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In CI, verify that PHPunit is actually running #50442

Merged
merged 7 commits into from
May 10, 2023

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented May 8, 2023

What?

This PR adds a check to the unit test workflow which makes sure that PHPunit is executing correctly.

Why?

In #50411, we found that all PHPUnit suites on trunk were not running at all. However, this did not fail the test suite, which meant the issue was undetected for more than a week.

How?

In the PHPUnit github workflow, add a grep call which extracts the number of completed tests from a phpunit log file, and makes sure that at least 500 tests are passing. (This is about half of the current test suite, which seems reasonable when we mostly want to make sure we're

There are a few different scenarios for test output:

  1. Tests do not run. The output looks like this:
ℹ Starting '/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose' on the tests-wordpress container. 

✔ Ran `/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose` in 'tests-wordpress'. (in 0s 701ms)
  1. Tests fail.
ℹ Starting '/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose' on the tests-wordpress container. 

(...a lot of output we don't care about...)

FAILURES!
Tests: 1181, Assertions: 2152, Failures: 3, Warnings: 34.
  1. Tests pass with warnings
(...a lot of output we don't care about...)
WARNINGS!
Tests: 1181, Assertions: 2156, Warnings: 34.
  1. Tests pass with no issues:
ℹ Starting '/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose' on the tests-wordpress container. 

(...a lot of output we don't care about...)

Time: 5.16 seconds, Memory: 58.00 MB

OK (1120 tests, 2156 assertions)
✔ Ran `/var/www/html/wp-content/plugins/gutenberg/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/gutenberg/phpunit.xml.dist --verbose` in 'tests-wordpress'. (in 6s 164ms)

The grep command looks for a match for OK (1120 tests, and then extracts the number from that and checks that it's more than 500. There is no match in the first two conditions, only in the third.

Testing Instructions

I tested this script in a bash script locally, but I'll also introduce revertable commits which cause an issue in CI so we can verify.

Testing Instructions for Keyboard

Screenshots or screencast

@noahtallen noahtallen self-assigned this May 8, 2023
@noahtallen noahtallen added [Type] Build Tooling Issues or PRs related to build tooling [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. GitHub Actions Pull requests that update GitHub Actions code and removed [Type] Build Tooling Issues or PRs related to build tooling labels May 8, 2023
@noahtallen noahtallen mentioned this pull request May 8, 2023
1 task
@noahtallen noahtallen requested review from ockham and gziolo May 8, 2023 18:38
@noahtallen noahtallen marked this pull request as draft May 8, 2023 19:13
@noahtallen noahtallen force-pushed the check-that-phpunit-runs branch from f3ba127 to a78aaff Compare May 8, 2023 19:14
@noahtallen noahtallen marked this pull request as ready for review May 8, 2023 21:24
@github-actions
Copy link

github-actions bot commented May 8, 2023

Flaky tests detected in bbbb38f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4919659420
📝 Reported issues:

Copy link
Contributor

@ObliviousHarmony ObliviousHarmony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work well.

The only thing I would say is that perhaps it makes sense for it to be included in the same step as the tests themselves. This doesn't seem like it will be a very common occurrence and so having a step feels like it might be superfluous in virtually all cases.

@noahtallen
Copy link
Member Author

This doesn't seem like it will be a very common occurrence and so having a step feels like it might be superfluous in virtually all cases.

The main reason I did it is just so the script didn't have to be duplicated in both tests, though we could put it in a bash script. IMO, having an extra step doesn't really hurt even if it's superfluous :P

@noahtallen noahtallen merged commit 3ee2290 into trunk May 10, 2023
@noahtallen noahtallen deleted the check-that-phpunit-runs branch May 10, 2023 21:38
@github-actions github-actions bot added this to the Gutenberg 15.9 milestone May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Actions Pull requests that update GitHub Actions code [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants