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 combination of `--format` & `--size_format` flags #118

Merged
merged 3 commits into from Oct 4, 2018

Conversation

2 participants
@austinginder
Copy link
Contributor

austinginder commented Oct 3, 2018

Fix #107

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 3, 2018

@austinginder Can you add Behat tests for the different cases to make sure this works as expected?

@austinginder

This comment has been minimized.

Copy link
Contributor

austinginder commented Oct 3, 2018

Sure thing. Will give it a shot.

@austinginder

This comment has been minimized.

Copy link
Contributor

austinginder commented Oct 3, 2018

Added a test, let me know if this looks correct. Thanks!

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Oct 4, 2018

@austinginder The approach is good, however you made the test reliable to a hardcoded size of the database. As you can see in the test output, this fails as soon as the version of WordPress changes.

Try changing the test from STDOUT should be to STDOUT should contain and omit the actual size. Basically, only test that there's more than just a number.

Example:

When I run `wp db size --size_format=b --format=csv`
Then STDOUT should contain:
  """
  Name,Size
  wp_cli_test,"
  """

This avoids hardcoding a specific magic number as size and thus makes the test less brittle.

@austinginder

This comment has been minimized.

Copy link
Contributor

austinginder commented Oct 4, 2018

Got it. I've updated the behat tests. Looks like things are passing now.

@schlessera schlessera added this to the 2.0.1 milestone Oct 4, 2018

@schlessera schlessera merged commit 7ffdd82 into wp-cli:master Oct 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@schlessera schlessera changed the title Fix issue #107 Fix combination of `--format` & `--size_format` flags Oct 4, 2018

@austinginder austinginder deleted the austinginder:107-consistent-db-label branch Nov 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment