fix: build one driver per run for read-only tests - #568
Open
jbmusso wants to merge 2 commits into
Open
Conversation
The file existence, file content and metadata runners in pkg/types/v2 each call NewDriver() and Destroy() inside their per-test loop. For the tar driver a driver IS an extracted image: NewTarDriver -> pkgutil.GetImageForName unpacks the whole image into a temporary directory, and Destroy deletes it again. An N-test run therefore unpacks the same image N times, and the cost grows with image size multiplied by test count. This adds a driver spy through the existing DriverImpl seam and asserts what a read-only run should cost: one driver, destroyed once. It reports 6 for a six-test run today, so this commit is red on its own and green after the next. Command tests are left alone: they mutate the container they run in, and their per-test driver lifecycle is what keeps them isolated from each other.
File existence, file content and metadata tests only read the image, so they can
all be answered by the same driver. Building one per test costs a full image
extraction per test under the tar driver, which makes a run cost image size
multiplied by test count instead of image size.
RunAll now builds a single driver for those three kinds and destroys it when they
are done. Each exported runner still stands on its own for a caller that drives
one kind directly: it builds a driver, uses it for every test of that kind, and
destroys it. SetEnv moves with the driver, so the run's global environment is
applied once instead of once per test.
Command tests are deliberately untouched. They mutate the container they run in,
and their per-test driver lifecycle is what isolates them from each other.
License tests are untouched for the same reason of keeping this change small.
Measured with the tar driver on a docker.io/library/node:24-slim docker-archive
(224 MiB, amd64, 6 file-existence tests, same 6 passes before and after;
extractions counted via --save and a fresh TMPDIR):
before: 6 extracttar dirs, 27s
after: 1 extracttar dir, 3.5s
The gap is the test count: six extractions become one.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Author
|
@googlebot I signed it! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Building a tar driver extracts the whole image, and the runner builds a driver per
test — so a run costs one full extraction per test. Six file-existence tests against
a
node:24-slimdocker-archive (224 MiB) leave sixextracttartemp dirs; verifiablewith
--saveand a freshTMPDIR. On multi-GB images this turns read-only suitesinto minutes, or CI timeouts.
Solution
test:a spy driver counting builds — 6 read-only tests build 6 drivers.Red at this commit:
go test ./pkg/types/v2/.fix:file existence, file content and metadata tests share one driver per run,destroyed when they finish;
SetEnvapplies once. Command tests keep theirper-test driver — they mutate the container, and that lifecycle is their
isolation. License tests are untouched to keep the change small.
Same run after the fix: one
extracttardir, same 6 passes, 27s → 3.5s wall time.AI-assisted analysis; the reproduction test, extraction counts and timings were
verified by hand.