-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
wp-env fix exec command in CI #50411
Conversation
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
Use -T instead of --no-TTY. The latter doesn't work. Fix unit test that was failing on trunk.
@@ -40,7 +40,7 @@ public function test_localizes_script() { | |||
); | |||
|
|||
$script = $wp_scripts->query( 'gutenberg-dummy-script', 'registered' ); | |||
$this->assertEquals( array( 'dependency', 'wp-i18n' ), $script->deps ); | |||
$this->assertEquals( array( 'dependency' ), $script->deps ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: in #49994, the function which injects wp-i18n was changed. According to @gziolo, this is expected:
I fixed [too many scripts loading] with 0cdab08. I'm not sure why that code was enforcing including wp-i18n as a dependency to all scripts coming from the Gutenberg plugin 🤷🏻
I replicated the logic from WordPress core that resolves the issue and ensures that translations still work where applicable
Since the phpunit tests were not running correctly, it makes sense that the phpunit tests were not updated in that PR
What?
Phpunit tests are not running anything in Github actions currently. This makes them work again. This was ultimately caused by #50007, so PHPunit hasn't been running on trunk or in branches for about a week. Thankfully, not many failures were introduced!
The issue was that wp-env would say phpunit was executing, but nothing would happen, and nothing would be printed to the terminal. Ultimately, Docker has very poor documentation around this issue (--no-TTY is supposed to work, but doesn't, and docker-compose run prints a help message, but docker-compose exec doesn't.) Plus, we don't have anything making sure that phpunit tests are being executed!
Essentially,
--no-TTY
isn't actually a flag that docker-compose accepts. When usingrun
instead ofexec
, you get a help message which says that only-T
is accepted. I guessed this was also the case for exec, but for some reason no help message was printed to indicate this. Changing that to-T
when we aren't running with a TTY (like in CI), it started running phpunit again.I also fixed the failing trunk test.
Follow-up
Testing Instructions
PHPunit should pass
Testing Instructions for Keyboard
Screenshots or screencast