Skip to content

Add comprehensive test coverage for utils and cmd packages - #32

Merged
kerren merged 5 commits into
developfrom
feature/testing-framework
Aug 30, 2026
Merged

Add comprehensive test coverage for utils and cmd packages#32
kerren merged 5 commits into
developfrom
feature/testing-framework

Conversation

@kerren

@kerren kerren commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Summary

This PR implements Phase T0, T2, and T3 of the testing plan outlined in docs/testing-plan.md, adding extensive test coverage for the utils and cmd packages. The changes include 13 new test files covering previously untested functions and adding CI hygiene guards.

Key Changes

  • New test files for utils package:

    • build_image_buildx_subprocess_test.go — 580 lines of subprocess integration tests using a fake docker shim to verify argument assembly, environment variable handling, credential isolation, and output streaming
    • hash_watch_directories_test.go — Tests for directory hashing with exclusion patterns
    • hash_watch_files_test.go — Tests for file hashing with order invariance
    • read_dockerignore_test.go — Tests for .dockerignore parsing and pattern handling
    • detect_buildx_test.go — Tests for buildx detection and version parsing
    • extract_version_test.go — Tests for version file extraction
    • hash_string_test.go — Tests for SHA256 hashing with known vectors
    • generate_docker_image_name_test.go — Tests for image name generation
    • parse_version_file_json_test.go — Tests for JSON version parsing
    • remove_empty_strings_from_array_test.go — Tests for array filtering
    • stdout_purity_test.go — Source-reading guard ensuring stdout remains pure outside write_build_output.go
  • New test file for cmd package:

    • readme_flag_coverage_test.go — Mechanical enforcement that every build flag is documented in README.md
  • CI hygiene:

    • .github/workflows/hygiene.yaml — New workflow that asserts no plan.md exists in the repository root (planning documents belong in docs/)
  • Documentation updates:

    • docs/testing-plan.md — Updated coverage percentages (utils: 38.2% → 52.6%, cmd: 0.0% → 33.8%) and marked completed phases

Notable Implementation Details

  • The subprocess tests in build_image_buildx_subprocess_test.go use a POSIX shell script fake docker executable to record argv, environment, and working directory without touching a real Docker daemon or registry
  • Tests are designed to be non-parallel where they manipulate process-global state (PATH, environment variables, os.Stdout/os.Stderr)
  • The stdout purity test uses regex scanning of source files to enforce the convention that only write_build_output.go writes to stdout
  • All new tests follow the existing pattern of pure unit tests (no registry, no credentials) except where subprocess interaction is required
  • Tests pin known edges and currently-silent behaviors documented in the testing plan, making them explicit rather than implicit

https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA

claude added 5 commits August 30, 2026 09:57
Part of Phase T2 in docs/testing-plan.md. Covers ParseVersionFileJson,
ExtractVersion, HashString, GenerateDockerImageName and
RemoveEmptyStringsFromArray, none of which had any coverage.

These pin current behaviour rather than change it, including the edges the
plan records as findings: ExtractVersion yields "v" for a version file of {}
and "vv1.0.0" for {"version":"v1.0.0"}, and RemoveEmptyStringsFromArray drops
"" but keeps " ".

Phase T2 is still in progress; the remaining hashing and dockerignore tests
follow in a later commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA
Covers the rest of Phase T2 in docs/testing-plan.md: HashWatchFiles,
HashWatchDirectories, ReadDockerignore, DetectBuildx and parseBuildxVersion.

Pins the contracts the plan calls load-bearing: HashWatchFiles and
HashWatchDirectories both return "" for an empty list, so a user who never
adopts the flags sees no change to cache identity, and DetectBuildx reports
every failure mode as (false, "", nil) rather than an error, leaving
ResolveBuilder to decide when that is fatal. ReadDockerignore's exclude
patterns are asserted to land after the file's, so a negation passed via
--exclude can re-include a file the .dockerignore excluded.

Also pins the in-place sort in HashWatchDirectories, which mutates the
caller's slice, as a documented finding rather than changing it here.

DetectBuildx is driven through a fake docker shim on PATH, so it needs
neither docker nor buildx to run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA
Phase T3 of docs/testing-plan.md. BuildImageBuildx was at zero coverage
despite carrying every rule in the "Subprocess credentials (buildx)" section
of CLAUDE.md.

Drives the function against a fake docker shim on PATH that records its argv,
environment and working directory. Asserts that DOCKER_CONFIG is set on the
subprocess environment only and never on dockem's own, that a pre-existing
DOCKER_CONFIG is stripped rather than duplicated, that every other parent
environment variable is passed through (which is what makes --secret
id=x,env=VAR work), that cmd.Dir is never set, that the throwaway config
directory is removed even when the build exits non-zero, and that subprocess
output lands on stderr so stdout stays clean for --output-format=json.

The password is checked against argv, BuildLog and BuildResult with a
sentinel value, so a future change that leaks it into any of the three fails
here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA
…rift

Phase T0 of docs/testing-plan.md.

Adds a Repo Hygiene workflow asserting no plan.md exists in the repository
root, case-insensitively, so planning documents stay in docs/ and scratch
state cannot reach develop or main. It runs on the same triggers as the unit
test workflow and is kept separate from it to stay cheap and readable.

Adds two source-reading guards in the spirit of the existing cache-hash
guards. The first fails if fmt.Print* or a direct os.Stdout write appears in
any non-test file outside write_build_output.go, enforcing the
LogInfo/LogWarn/LogError convention that keeps stdout carrying nothing but
the JSON result. The second fails if a flag registered in cli/cmd/build.go is
missing from the README, enforcing the documented convention that the two
stay in sync. This is the first test file in the cmd package.

Both guards were verified to fail on an injected violation before landing;
cobra's auto-generated --version flag is carried as a documented exception
rather than by editing the README.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA
T0, T2 and T3 have landed. Coverage on a non-registry run is now 52.6% for
utils (from 38.2%) and 33.8% for cmd (from zero).

One T0 item is deliberately left open: confirming the root plan.md guard
actually goes red in real CI needs a scratch-branch push to observe, which
cannot be done from a local run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tn4Aqbe88wWU6a5njxB8dA
@kerren
kerren merged commit ea4a623 into develop Aug 30, 2026
3 checks passed
@kerren
kerren deleted the feature/testing-framework branch August 30, 2026 10:24
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.

2 participants