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.

Sign up for GitHub

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

Jump to bottom

[healthcare] fix: fix broken tests for healthcare API #3253

Merged
merged 7 commits into from
Apr 3, 2020
Merged

[healthcare] fix: fix broken tests for healthcare API #3253

merged 7 commits into from
Apr 3, 2020

Conversation

tmatsuo
Copy link
Contributor

@tmatsuo tmatsuo commented Apr 1, 2020 •
edited
Loading

fixes #3120
fixes #3134

@tmatsuo tmatsuo requested review from leahecole and busunkim96 April 1, 2020 23:16
@tmatsuo tmatsuo requested review from noerog and a team as code owners April 1, 2020 23:16
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 1, 2020
@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 1, 2020

It seems like the API changed a bit, so I needed to make some tweak. I added @busunkim96 and @leahecole as reviewers, but let me know if I'm doing wrong.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 2, 2020

Wow. It seems like those 2 builds are using the same resource name. I think this is a recipe for disaster. I'll also fix it in the next commit.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 2, 2020

It's still flaky:

Traceback (most recent call last):
  File "/tmpfs/src/github/python-docs-samples/healthcare/api-client/hl7v2/hl7v2_messages_test.py", line 98, in test_CRUD_hl7v2_message
    hl7v2_message_name = hl7v2_messages_list[0].get('name')
IndexError: list index out of range

As far as I read the (log)[https://source.cloud.google.com/results/invocations/3cdf88e2-52f8-47e0-a757-a2792429c141/log], the test successfully created a new message and the subsequent list call returned an empty result.

Maybe I can retry for several times.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 2, 2020

@busunkim96 I ran the tests locally 10 times and all passed. PTAL

@tmatsuo
Copy link
Contributor Author

tmatsuo commented Apr 2, 2020

Adding .gitignore causes the builds running all the tests :( I'll revert it

gguuss
gguuss approved these changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

LGTM, great to see these tests green.

gguuss
gguuss approved these changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

LGTM, great to see these tests green.

gguuss
gguuss approved these changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

LGTM, great to see these tests green.

@leahecole
Copy link
Collaborator

@noerog PTAL - it's a LGTM from me, but given that you're the codeowner, I just want your final stamp :)

@noerog
Copy link
Contributor

noerog commented Apr 3, 2020

LGTM, thank you @tmatsuo

@tmatsuo tmatsuo added the automerge Merge the pull request once unit tests and other checks pass. label Apr 3, 2020
@tmatsuo tmatsuo merged commit f8a1d3f into GoogleCloudPlatform:master Apr 3, 2020
@tmatsuo tmatsuo deleted the healthcare-fix branch April 3, 2020 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@busunkim96 busunkim96 busunkim96 left review comments

@noerog noerog noerog approved these changes

@gguuss gguuss gguuss approved these changes

@leahecole leahecole Awaiting requested review from leahecole

Assignees

@crwilcox crwilcox

Labels
automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
7 participants
@tmatsuo @leahecole @noerog @gguuss @busunkim96 @crwilcox @googlebot