-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix failing PHPUnit tests #188
Conversation
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.
Some nitpicks. Rest LGTM!
.phpunit.cache | ||
.phpunit.result.cache |
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.
.phpunit.cache | |
.phpunit.result.cache | |
*.[Cc]ache |
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.
I personally prefer the more verbose format
beStrictAboutTestsThatDoNotTestAnything="true" | ||
beStrictAboutTodoAnnotatedTests="true" | ||
colors="true" | ||
verbose="true"> |
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.
We might want to remove it from here, as PHPUnit doesn't support it in future releases. Verbosity can be handled from CLI instead if required?
beStrictAboutTodoAnnotatedTests="true" | ||
colors="true" | ||
verbose="true"> | ||
<testsuite name="wp-cli/package-command tests"> |
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.
<testsuite name="wp-cli/package-command tests"> | |
<testsuite name="default"> |
It can remain default given this test file is specific to this package. Also, I think we should add defaultTestSuite="default"
attribute in the <phpunit>
element?
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.
This is more consistent with other commands though. Main thing is that the name
is defined, which it previously wasn't.
Fixes compatibility with latest PHPUnit on PHP 8.1+ after wp-cli/wp-cli-tests#206