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

Add server-side recording and test replay #101

Merged
merged 7 commits into from
Aug 19, 2024

Conversation

calvinmclean
Copy link
Contributor

Description

Rather than mocking/recording external HTTP interactions, this enables recording and replaying incoming interactions with your application's HTTP server.

This works by putting the Recorder in a middleware and uses the existing code for recording request/response. Instead of using a RoundTripper to get the response, the middleware provides the underlying handler's actual response.

Then, a few methods are added to the cassette package for replaying and testing the recorded interactions.

No breaking changes.

More details

Hello! Thanks for creating such a great library for testing HTTP clients.

I have a need for doing something similar on the server-side to record interactions and convert them into regression tests. Initially I was able to create a separate library that wraps the Recorder to achieve this, but realized it will be better if I integrate it directly, especially after seeing #88.

If I create this as a separate library/package, then I have to use rec.SetRealTransport for each request in the middleware which likely isn't safe for concurrent use.

I expect to receive some feedback and discussion as part of this PR, so I am open to any suggestions on how to make this a better fit for the library.

Lastly, no hard feelings if you choose to decline this PR 🙂

@calvinmclean calvinmclean changed the title Add server-side recording and test replay [draft] Add server-side recording and test replay Aug 12, 2024
@calvinmclean
Copy link
Contributor Author

renamed to draft since this likely needs more tests

@calvinmclean calvinmclean marked this pull request as draft August 12, 2024 04:27
@calvinmclean calvinmclean changed the title [draft] Add server-side recording and test replay Add server-side recording and test replay Aug 12, 2024
@dnaeon
Copy link
Owner

dnaeon commented Aug 12, 2024

Hey @calvinmclean ,

Nice PR, thanks!

Let me know when it's ready for review and I'll check it out :)

@calvinmclean calvinmclean marked this pull request as ready for review August 18, 2024 00:30
@calvinmclean
Copy link
Contributor Author

@dnaeon, I made a few improvements and added more tests so it should be ready for review!

@dnaeon
Copy link
Owner

dnaeon commented Aug 18, 2024

Hey @calvinmclean ,

Thanks again! Could you please rebase the PR against the latest changes in v3? I've added 1.22.x and 1.23.x versions to be used for testing, which this PR requires.

@dnaeon
Copy link
Owner

dnaeon commented Aug 18, 2024

Hey @calvinmclean ,

I've just pushed v4 branch in preparation for the next release of go-vcr. Can you please target v4 branch for this PR, so that we can include in it? Thanks!

EDIT-1: Actually, nevermind, lets use v3 branch for this PR, as there may be breaking changes in v4 while I'm working on it, so I'll make sure to transfer these to the next branch.

EDIT-2: @calvinmclean , I've just completed with the refactoring for next v4 version of go-vcr.

There were some long-standing things I wanted to clean up from the code base, which I just did.

Would you mind refactoring the code against the v4 branch? Since this is going to be a new major release, I think we can afford breaking changes, if needed.

@calvinmclean
Copy link
Contributor Author

@dnaeon thanks! I'll rebase shortly. I can also change the header matching function to try and reuse some of the existing matcher instead of requiring Go 1.22 features for maps and slices to preserve some compatibility. What do you think?

@dnaeon
Copy link
Owner

dnaeon commented Aug 18, 2024

@calvinmclean , I've just set the minimum version of Go to 1.22 in v4/go.mod.

I think you should probably use whatever is provided in the standard lib as part of 1.22 and later, that makes the code more idiomatic.

Thanks!

@dnaeon
Copy link
Owner

dnaeon commented Aug 18, 2024

@calvinmclean , something else that I was thinking about is whether to call the Middleware method HTTPHandler, HTTPMiddleware or something similar to make it clearer that this is HTTP-specific thing, that is supposed to be used server-side. What do you think?

@calvinmclean
Copy link
Contributor Author

@calvinmclean , something else that I was thinking about is whether to call the Middleware method HTTPHandler, HTTPMiddleware or something similar to make it clearer that this is HTTP-specific thing, that is supposed to be used server-side. What do you think?

Yeah good idea! I'll use HTTPMiddleware

- Use a server-side middleware to record requests from an external
  client and record the server's response
- Replay these recorded interactions to test that the server produces
  the same responses
@calvinmclean
Copy link
Contributor Author

@dnaeon
Rebased and pushed with renamed HTTPMiddleware.

The new options pattern in v4 is a nice improvement!

@calvinmclean calvinmclean changed the base branch from v3 to v4 August 18, 2024 17:30
@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 14.66667% with 64 lines in your changes missing coverage. Please review.

Project coverage is 51.91%. Comparing base (3873f09) to head (5ae4774).
Report is 9 commits behind head on v4.

Files Patch % Lines
pkg/cassette/server_replay.go 0.00% 37 Missing ⚠️
pkg/recorder/middleware.go 0.00% 23 Missing ⚠️
pkg/cassette/cassette.go 50.00% 1 Missing and 1 partial ⚠️
pkg/recorder/recorder.go 81.81% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##               v4     #101      +/-   ##
==========================================
- Coverage   59.01%   51.91%   -7.11%     
==========================================
  Files           2        4       +2     
  Lines         427      497      +70     
==========================================
+ Hits          252      258       +6     
- Misses        156      218      +62     
- Partials       19       21       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

README.md Outdated Show resolved Hide resolved
@dnaeon dnaeon merged commit 3916d5f into dnaeon:v4 Aug 19, 2024
2 checks passed
@dnaeon
Copy link
Owner

dnaeon commented Aug 19, 2024

Hey @calvinmclean , thanks!

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.

3 participants