test(worker-image): cover ustar tar-builder + count integration coverage in codecov (MNG-1726)#1477
Merged
zbigniewsobiecki merged 1 commit intoJul 3, 2026
Conversation
…age in codecov (MNG-1726)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
nhopeatall
approved these changes
Jul 2, 2026
nhopeatall
left a comment
Collaborator
There was a problem hiding this comment.
Summary
APPROVE — a clean, tightly-scoped coverage-hardening PR that fully satisfies the MNG-1726 acceptance criteria with no product-behavior change. I verified the substance rather than trusting the description.
What I verified
- v8-ignore markers are narrow and correctly placed. In both
worker-image-build.tsandworker-image-validation.tsthe/* v8 ignore start|stop */blocks wrap only thedefaultDepsdaemon-glue functions.createSingleFileTar,resolveDigestFromRepoDigests,composeDockerfile, all handler logic, and thedefaultDepsobjects remain outside the markers and stay counted — matching the ticket's "do NOT blanket-ignore" constraint. - The exclusions drop no real coverage signal. The excluded functions are referenced in tests only as
vi.fn()mocks (injected fakes); none is invoked directly, so nothing testable is being hidden. - The tar test is a genuine round-trip, not a tautology. I traced every assertion against the actual builder — header field byte-offsets, the space-filled 512-byte checksum recomputation, block padding (incl. exact-multiple and empty-file cases), the 1024-byte trailer, and the multibyte-UTF-8 byte-accurate size — and the hand-rolled
parseUstarreader is independent of the producer. Ran it locally: 19/19 pass in 5ms. The mtime test re-creates a fresh tar bounded bybefore/after, so it isn't flaky. - Codecov wiring is sound. Two separate CI jobs upload
coverage/lcov.infounderunit/integrationflags (the vitest root coverage config emits lcov). Codecov merges flags as a per-line union, so integration-only-covered repository code (theprojectsRepositorywriters) counts toward patch/project without any risk of lowering the combined number. Flags are declared withcarryforward: true; no new per-flag blocking status is introduced, andcodecov.ymlis well-formed. - Hygiene/scope. Exactly 6 intentional files, no generated artifacts; source edits are comment-only; CI is 7/7 green.
No blocking or should-fix issues.
🕵️ claude-code · claude-opus-4-8 · run details
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
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.
Summary
Coverage-hardening follow-up to the merged per-project worker Dockerfile sequence (MNG-1721 → MNG-1725). PR #1474 (MNG-1723) merged with
codecov/patchred because the gaps were integration-tested code + un-mockable Docker glue rather than untested logic. This PR closes those gaps properly and establishes a durable codecov policy so future image/build PRs don't trip the same carve-out.Scope: test + CI/coverage config only. No product-behavior changes — no build/spawn/set/UI surface is touched.
Linear: https://linear.app/mongrel/issue/MNG-1726/worker-dockerfile-follow-ups-test-coverage-hardening-codecov-policy
What changed
1. Direct unit test for
createSingleFileTar(the hand-rolled ustar builder)tests/unit/router/worker-dockerfile-tar.test.ts(new, 19 tests) asserts the POSIX ustar output of the riskiest hand-written code in the sequence:ustar@257, version @263.Dockerfileentry whose bytes equal the composed Dockerfile, including a multibyte-UTF-8 byte-accurate size case.createSingleFileTarwas already exported, so no product code changed for this.2. Count integration coverage in Codecov (closes the
projectsRepository0%-patch gap)recordWorkerImageBuildResult/readWorkerImageBuildInputsare thoroughly tested bytests/integration/db/projectsRepository.test.ts, but they read as 0% patch because only the unit run was uploaded to Codecov. Rather than paper over just these two functions, this wires the integration coverage into Codecov so all integration-covered repository/db code counts:package.json— newtest:integration:coveragescript runs the integration project with--coverageand thresholds disabled (the integration suite alone doesn't exercise 80% ofsrc/**; the combined Codecov report is the gate, not the local run)..github/workflows/ci.yml— theintegration-testsjob now runstest:integration:coverageand uploadscoverage/lcov.infounder a newintegrationflag; the existing unit upload gains aunitflag. Codecov merges the two flags per-commit.codecov.yml— declares theunit+integrationflags withcarryforward: true.Verified locally against a live DB: the integration lcov records hits > 0 on every executable line of both writers (
readWorkerImageBuildInputs12/12,recordWorkerImageBuildResult30/30).3. Durable, narrowly-scoped policy for un-mockable Docker-daemon glue
The
defaultDepsreal-Docker implementations only run against a live daemon, so unit tests drive the handlers through injected fakes and the runtime contract is covered by the shared worker-image smoke-test (docker-build-checkCI job). Those specific default-impl blocks now carry line-level/* v8 ignore start | stop */markers so they can't recurrently failcodecov/patchon future image/build PRs:src/router/worker-image-build.ts—defaultResolveBaseDigest,defaultBuildImage,defaultInspectBuiltImage.src/router/worker-image-validation.ts—defaultInspectImageDigest,runWorkerImageSmokeTest.codecov.ymldocuments the policy. This is deliberately narrow: the handler logic, compose/hashing helpers, andcreateSingleFileTarstay fully counted — the files are not blanket-ignored. Scoped coverage confirms the build handler now reports 100% function coverage with only pre-existing handler branches (not the glue) uncovered.Verification
npm run typecheck— clean.biome checkon changed files — clean.npm run test:coverage(full unit suite) — 10687 passed / 33 skipped, exit 0, thresholds pass (global lines 92.61%).npm run test:integration:coverage— 649 passed, exit 0,coverage/lcov.infoproduced.tests/unit/router/worker-dockerfile-tar.test.ts— 19 passed.Out of scope
No behavior change to the build engine / spawn / set surfaces / UI. No registry push / multi-host / build-time secrets / base-image-bump work.
🤖 Generated with Claude Code
🕵️ claude-code · claude-opus-4-8 · run details