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

Allow for `installation` as well as `install` #177

Merged
merged 2 commits into from Oct 5, 2018

Conversation

2 participants
@henrywright
Copy link
Contributor

henrywright commented Oct 3, 2018

Related to #4541 in wp-cli.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 3, 2018

@henrywright The tests now fail because the change is not yet in.

As already suggested in wp-cli/wp-cli#4960 (comment), the preferred fix is to just remove the period (.) from the end of the sentence.

As the test says STDERR should *contain*, removing the period will match both install and installation at the same time and make the test less brittle.

This applies to all 5 PRs you've introduced to that regard.

@henrywright

This comment has been minimized.

Copy link
Contributor

henrywright commented Oct 3, 2018

Hey @schlessera

Thanks for the feedback. I am currently matching the string exactly. Happy to amend if needed.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 3, 2018

@henrywright Yes, please amend. As you can see, the tests fail now, as they only work for the new version, but not for the old anymore (which is still run through these same tests).

@henrywright

This comment has been minimized.

Copy link
Contributor

henrywright commented Oct 4, 2018

I have now amended the relevant tests so that they will match both "install" and "installation" in standard error output.

That should fix test failures in the old version. Let me know if anything else is needed.

@schlessera schlessera added this to the 2.0.2 milestone Oct 5, 2018

@schlessera schlessera changed the title Use "installation" instead of "install" Allow for `installation` as well as `install` Oct 5, 2018

@schlessera schlessera merged commit ad4bbc9 into wp-cli:master Oct 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment