test: Faster tests by being consistent with what counts as TEST_ACCEPTANCE

[another-rex]

This makes the standard tests (make test) run under 5s by making the following changes:

• Split up TEST_ACCEPTANCE into two things,
  • TEST_ACCEPTANCE is now purely for tests that require external dependencies (or extreme reqs like high memory)
  • go test -short is now for tests that run under 5s (which is the majority of the tests, including container scanning tests)

Moved all of the following tests to be skipped if -short is passed

• Offline database tests

• Java archive reachability tests (does dep resolution)

• Transitive dependency tests

• Moved all docker dependency tests under TEST_ACCEPTANCE=true

• Some tests have both TEST_ACCEPTANCE=true and SkipIfShort, this allows for short tests while TEST_ACCEPTANCE=true

  • Example is OCI container image tests, which require docker to build, but runs very fast.

This also includes some additional minor changes to aid with debugging and testing.

• Print out the stdout and stderr if the error code is not expected. Right now, especially for the tests with JSON replace rules, it just fails without any detail.
• Also for JSON replace tests, print out the stdout and stderr messages when json fail to parse.
• Make sure to sort keys when writing cassettes to disk
• Made container scanning tests with custom plugin flags actually test stuff, and run much faster.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 16.66667% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.74%. Comparing base (c82c9c9) to head (23f1eac).

Files with missing lines	Patch %	Lines
cmd/osv-scanner/internal/testcmd/run.go	0.00%	3 Missing and 1 partial ⚠️
cmd/osv-scanner/internal/testcmd/vcr.go	0.00%	4 Missing ⚠️
internal/testutility/utility.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2382      +/-   ##
==========================================
- Coverage   67.75%   67.74%   -0.02%     
==========================================
  Files         172      172              
  Lines       13291    13296       +5     
==========================================
+ Hits         9005     9007       +2     
- Misses       3581     3582       +1     
- Partials      705      707       +2     

☔ 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.

[G-Rath]

So the idea with TEST_ACCEPTANCE was about tests that had exotic dependencies such as Docker or the Rust build chain, not (initially) about speed, and imo the test suite should aim to run as many of our tests as possible by default.

I think rather than change that, we should make proper use of testing.Short(), probably with a helper like testutility.IsSlow, so that you can then run the tests fast with go test -short ./...

[cuixq]

My understanding is that TEST_ACCEPTANCE is for tests that require additional dependencies but not for tests that are slow to run. Shall we have a separate env variable for the slow tests?

[another-rex]

Huh did not know about the -short flag, will look into that.

This change actually makes TEST_ACCEPTANCE actually cut out tests that require external dependency (there's a few docker tests that were not behind the flag),

I'll do another pass to make sure only tests that require external deps are behind the flag.

[G-Rath]

(there's a few docker tests that were not behind the flag),

despite what I've said, I actually don't think we should consider Docker as exoitc since I think thats a very common dependency and it would mean all of our image scanning tests get disabled by default.

I think if we use -short, we get the nice layers that we want:

• TEST_ACCEPTANCE=false go test -short ./... for running the suite fast (aka without any external dependencies, including Docker)
• TEST_ACCEPTANCE=false go test ./... for running the suite with some external dependencies that we consider common (e.g. Docker and HTTP)
• TEST_ACCEPTANCE=true go test ./... for running everything including tests that require specific toolchains like Rust

[another-rex]

There's a misunderstanding here, we have two sets of docker tests, one for with the --archive flag, and one without. The one without --archive actually just shells out for docker save blahblah > temp.tar and then essentially runs osv-scanner scan image --archive temp.tar.

So leaving them out of test acceptance is no issue, since it's really just testing some very thin convenience code.

In this PR I actually enabled image scanning with --archive when TEST_ACCEPTANCE=false, since it runs really fast with vcrs now.

[another-rex]

Ok, made a few more changes:

Split up TEST_ACCEPTANCE into two things,

TEST_ACCEPTANCE is now purely for tests that require external dependencies (or extreme reqs like high memory)

go test -short is now for tests that run under 5s (which is the majority of the tests, including container scanning tests)
Made container scanning tests with custom plugin flags actually test stuff, and run much faster.

PTAL @cuixq @G-Rath