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

just a small refactor of api_test #291

Merged
merged 3 commits into from Mar 16, 2023

Conversation

zevdg
Copy link
Collaborator

@zevdg zevdg commented Sep 8, 2022

This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation (namely RegisterTestRequest).

This change was originally part of #284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.

@zevdg zevdg force-pushed the register_api_test_request branch 2 times, most recently from 85c0a20 to 492ceb5 Compare September 8, 2022 23:21
This makes the test more hermetic by avoiding the need to set env vars, and it also avoids some unecessary duplication of test helper logic by leveraging some of aetest's underlying implementation.

This change was originally part of golang#284, but I split it out because it's not compatible with v1's log flushing tests, and it would have added unecessary noise to that PR.
Copy link
Collaborator

@jinglundong jinglundong left a comment

Choose a reason for hiding this comment

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

Thanks!

@ludoch
Copy link
Collaborator

ludoch commented Mar 15, 2023

Can you resolve the conflicts we see now?

@zevdg
Copy link
Collaborator Author

zevdg commented Mar 16, 2023

oh, whoops. i didn't realize this was approved but unmerged.

@zevdg
Copy link
Collaborator Author

zevdg commented Mar 16, 2023

huh, i resolved the conflicts in the github UI, but now the tests are failing because python2 is missing, so I don't have full confidence that I didn't mess something up in the merge...

@ludoch
Copy link
Collaborator

ludoch commented Mar 16, 2023

I've a PR in review. Once in, I will merge yours to be sure.
#304

@ludoch ludoch merged commit 28bf581 into golang:master Mar 16, 2023
1 of 20 checks passed
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.

None yet

3 participants