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

ensures servicebuilder throws proper exceptions #682

Merged

Conversation

bshaffer
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 21, 2017
Copy link
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

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

I could have sworn this was throwing exceptions when I last reviewed it 😕. Either way, lgtm!

@bshaffer
Copy link
Contributor Author

bshaffer commented Sep 25, 2017

It was, but then the service builder was changed and I must have goofed the rebase. I'm more curious why the tests didn't fail.

@bshaffer
Copy link
Contributor Author

The code coverages are inaccurate... I believe it is because the system test (tests/system/Core/ServicesNotFoundTest.php) is not being run.

@bshaffer
Copy link
Contributor Author

bshaffer commented Sep 25, 2017

Ahh yes, the system tests are only run on the cron. I moved the test from tests/system to tests/unit, so this should be fixed.

@bshaffer bshaffer force-pushed the ensure-servicebuilder-throw-exception branch from 85767dc to a4708bc Compare September 25, 2017 22:32
@dwsupplee dwsupplee merged commit 36124dd into googleapis:master Sep 26, 2017
@jdpedrie jdpedrie mentioned this pull request Oct 16, 2017
@bshaffer bshaffer deleted the ensure-servicebuilder-throw-exception branch April 29, 2021 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants