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

Use EventuallyConsistentTestTrait for a test #186

Merged
merged 5 commits into from
Oct 11, 2016

Conversation

tmatsuo
Copy link
Contributor

@tmatsuo tmatsuo commented Oct 11, 2016

No description provided.

@@ -4,7 +4,8 @@
"symfony/console": "^3.0"
},
"require-dev": {
"phpunit/phpunit": "~4.8"
"phpunit/phpunit": "~4.8",
"google/cloud-tools": "<2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an odd way to declare this. We should start using semantic versioning and use ^1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

I realize this won't work, since our highest version is 0.5. I think I see what you're going for, you want to basically say we'll maintain BC up to 2.0. I wonder if there's a better way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might seem odd, but it's sane to me. Here is the reason.

Now google/cloud-tools is around 0.5. We can not use ^1.0 because 0.5 doesn't match it. Sometimes in the future, I may release the official version 1.0 of that tool. After that, it should follow semantic versioning. I don't want this test to break when I release google/cloud-tools 2.0. So I'm using <2.0.

],
['interactive' => false]
);
$this->expectOutputRegex('/: Test Message/');
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work as designed, as expectOutputRegex is only assessed at the end of the test's execution. Instead you'll have to capture the output:

ob_start();
$commandTester->execute(
    [
        '--project' => $this->projectId,
        '--logger' => 'my_test_logger'
    ],
    ['interactive' => false]
);
$output = ob_get_clean();
$this->assertContains(':Test Message', $output);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I'll fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@tmatsuo tmatsuo force-pushed the logging-eventual-consistent-runner branch from 8f32a48 to f4ed894 Compare October 11, 2016 20:23
@tmatsuo
Copy link
Contributor Author

tmatsuo commented Oct 11, 2016

Use ob_start and ob_get_clean. Also rebased to the master. @bshaffer PTAL

@tmatsuo tmatsuo merged commit a98a469 into master Oct 11, 2016
@tmatsuo tmatsuo deleted the logging-eventual-consistent-runner branch October 11, 2016 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants