Skip to content

test: fail fast on missing VCR cassettes by converting errors to 404#2816

Merged
another-rex merged 1 commit into
google:mainfrom
another-rex:fail-missing-cassette-fast-20260518
May 20, 2026
Merged

test: fail fast on missing VCR cassettes by converting errors to 404#2816
another-rex merged 1 commit into
google:mainfrom
another-rex:fail-missing-cassette-fast-20260518

Conversation

@another-rex
Copy link
Copy Markdown
Collaborator

@another-rex another-rex commented May 18, 2026

If a VCR cassette or a specific interaction is missing, the OSV client would previously retry the request 4 times with exponential backoff, causing significant delays in test execution (over 20 seconds).

This change wraps the VCR client transport in tests to intercept the "requested interaction not found" error (using errors.Is with cassette.ErrInteractionNotFound for robust matching) and convert it to a "404 Not Found" response. Since the OSV client does not retry on 404 status codes, this allows tests to fail immediately (~0.02s) when a cassette or interaction is missing while still preserving retry behavior for genuine transient network errors.

This approach avoids modifying the core osvscanner library API or adding new CLI flags.

  • Implemented vcrErrorWrappingTransport in internal/testcmd/vcr.go
  • Wrapped the VCR client transport in InsertCassette
  • Used errors.Is with cassette.ErrInteractionNotFound for robust error matching

@another-rex another-rex force-pushed the fail-missing-cassette-fast-20260518 branch from b15ead4 to 7ab4e88 Compare May 18, 2026 07:13
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.16%. Comparing base (f9ffc38) to head (78c17de).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2816      +/-   ##
==========================================
+ Coverage   79.14%   79.16%   +0.01%     
==========================================
  Files         121      121              
  Lines        8185     8191       +6     
==========================================
+ Hits         6478     6484       +6     
  Misses       1324     1324              
  Partials      383      383              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@another-rex another-rex force-pushed the fail-missing-cassette-fast-20260518 branch from 7ab4e88 to 78c17de Compare May 19, 2026 00:19
@another-rex another-rex changed the title Disable network retries in tests to fail fast on missing cassettes test: disable network retries in tests to fail fast on missing cassettes May 19, 2026
@another-rex another-rex requested a review from cuixq May 19, 2026 03:18
if maxNetworkQueryAttempts > 0 {
// osvdev.Config.MaxRetryAttempts actually controls the maximum number of attempts,
// where 1 means 1 attempt (0 retries).
config.MaxRetryAttempts = maxNetworkQueryAttempts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am a bit confused about how this config will be propagated through? is the config defined in osv-scalibr?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's not needed for scalibr, we have not switched to use the osv-scalibr osvdev enrichers yet.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But I think I'll rework this a bit, I'm not sure we want to have this as a config in general.

@another-rex another-rex force-pushed the fail-missing-cassette-fast-20260518 branch from 78c17de to 9d7170c Compare May 19, 2026 05:41
@another-rex another-rex changed the title test: disable network retries in tests to fail fast on missing cassettes test: fail fast on missing VCR cassettes by converting errors to 404 May 19, 2026
@another-rex another-rex requested a review from cuixq May 19, 2026 05:43
Comment thread cmd/osv-scanner/internal/testcmd/vcr.go Outdated

func (t *vcrErrorWrappingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
resp, err := t.wrapper.RoundTrip(req)
if err != nil && strings.Contains(err.Error(), "requested interaction not found") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use errors.Is to check the error?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, done.

If a VCR cassette or a specific interaction is missing, the OSV client
would previously retry the request 4 times with exponential backoff,
causing significant delays in test execution (over 20 seconds).

This change wraps the VCR client transport in tests to intercept the
"requested interaction not found" error (using errors.Is with
cassette.ErrInteractionNotFound) and convert it to a "404 Not Found"
response. Since the OSV client does not retry on 404 status codes, this
allows tests to fail immediately (~0.02s) on missing cassettes/interactions
while still preserving retry behavior for genuine transient network errors.

- Implemented vcrErrorWrappingTransport in internal/testcmd/vcr.go
- Wrapped the VCR client transport in InsertCassette
- Used errors.Is with cassette.ErrInteractionNotFound for robust error matching
@another-rex another-rex force-pushed the fail-missing-cassette-fast-20260518 branch from 9d7170c to f4df75a Compare May 20, 2026 03:12
@another-rex another-rex requested a review from cuixq May 20, 2026 03:42
@another-rex another-rex merged commit ab572ec into google:main May 20, 2026
15 of 18 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.

3 participants