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

Improve record/replay tests #1492

Merged
merged 8 commits into from
May 20, 2021
Merged

Improve record/replay tests #1492

merged 8 commits into from
May 20, 2021

Conversation

Porges
Copy link
Member

@Porges Porges commented May 18, 2021

A few changes here:

Check bodies in requests

This makes sure the request/response pair is for the correct request (by default, go-vcr only checks the HTTP method/URL). To achieve this we blank out any dates/timestamps that are found in bodies, since they cause a mismatch when the test is rerun.

Fix "count" header addition

We had code to track the request count at a per-URL level so that we handle multiple requests to the same URL properly in recordings (so that a series of polling GETs can get back different responses, for example), but this was not being added properly. Now it is.

Better messages when go-vcr interaction cannot be found

At the moment when running record/replay tests and an interaction (request+response) cannot be located, it is hard to figure out what went wrong. This PR adds a wrapper around the go-vcr code to add more context about what went wrong.

An example message in the output looks like:

*** Cannot find go-vcr recording for request (no responses recorded for this method/URL): DELETE https://management.azure.com/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/k8sinfratest-rg-kjqcwl?api-version=2020-06-01

Ideally we would panic when this happens, but we can't at the moment (the code explains why).

Closes #1432.

How does this PR make you feel:
VCR error

@Porges Porges force-pushed the better-failure-messages branch from b7e3f61 to b4da5b4 Compare May 18, 2021 22:01
@Porges Porges changed the title Better messages when interaction cannot be found Better messages when go-vcr interaction cannot be found May 18, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 18, 2021

Codecov Report

Merging #1492 (40661b9) into master (b790d84) will increase coverage by 0.07%.
The diff coverage is 59.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1492      +/-   ##
==========================================
+ Coverage   62.13%   62.20%   +0.07%     
==========================================
  Files         161      163       +2     
  Lines       10695    10730      +35     
==========================================
+ Hits         6645     6675      +30     
- Misses       3414     3415       +1     
- Partials      636      640       +4     
Impacted Files Coverage Δ
...erated/pkg/testcommon/kube_test_context_envtest.go 84.00% <ø> (+8.09%) ⬆️
hack/generated/pkg/testcommon/test_context.go 58.82% <52.94%> (+3.94%) ⬆️
...d/pkg/testcommon/error_translating_roundtripper.go 53.12% <53.12%> (ø)
.../generated/pkg/testcommon/counting_roundtripper.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b790d84...40661b9. Read the comment docs.

Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

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

LGTM - left a few comments/questions though.

@Porges Porges changed the title Better messages when go-vcr interaction cannot be found Improve record/replay tests May 19, 2021
@Porges Porges merged commit 685ad94 into master May 20, 2021
@Porges Porges deleted the better-failure-messages branch May 20, 2021 22:31
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.

Need better error if recording tests body mismatches
3 participants