Skip to content

Refactor TestGetAuthorities into table-driven subtests - #1997

Open
nellshamrell wants to merge 2 commits into
sigstore:mainfrom
nellshamrell:table-driven-getauthorities-test
Open

Refactor TestGetAuthorities into table-driven subtests#1997
nellshamrell wants to merge 2 commits into
sigstore:mainfrom
nellshamrell:table-driven-getauthorities-test

Conversation

@nellshamrell

Copy link
Copy Markdown
Contributor

Summary

Converts the monolithic TestGetAuthorities in pkg/apis/config/image_policies_test.go into named, table-driven subtests, resolving the long-standing in-code TODO:

// TODO: Clean up this test to be table-driven with sub-tests, instead of one big test.

Motivation

The original test ran ~15 independent scenarios sequentially against one shared config. A single t.Fatalf (e.g. via the getAuthority helper) aborted all remaining assertions, and failures reported generic messages with no indication of which scenario broke.

Changes

  • Each scenario now runs as its own t.Run subtest, so failures are isolated and clearly named (--- FAIL: TestGetAuthorities/keyless_authority, etc.).
  • Homogeneous cases (single key-authority matches, resource matching) are grouped into small tables; heterogeneous cases (keyless, multiple-matches, attestations, source-oci, signature-pull-secrets) are individual named subtests.
  • Panic-prone dereferences (nil Keyless.InsecureIgnoreSCT, indexing into Identities/Sources/Attestations/SignaturePullSecrets) are guarded so a malformed fixture yields a clean subtest failure instead of a panic.

Behavior preservation

This is a test-only, behavior-preserving refactor: the same assertions run with the same expected values as before. No production code is touched.

Testing

go test ./pkg/apis/config/
go vet ./pkg/apis/config/
gofmt -l pkg/apis/config/image_policies_test.go

All pass / clean.

Convert the monolithic TestGetAuthorities into named, table-driven
subtests, resolving the long-standing TODO in image_policies_test.go.

Each scenario now runs as its own t.Run subtest, so a failure reports
which case broke and no longer aborts the remaining assertions. Panic-
prone pointer/slice dereferences (nil Keyless.InsecureIgnoreSCT, indexing
into Identities/Sources/Attestations/SignaturePullSecrets) are guarded so
malformed fixtures produce clean failures instead of a panic that kills
the whole test.

This is a behavior-preserving, test-only change: the same assertions run
with the same expected values as before.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0ce53efa-a7e9-4046-a24e-34c406601950
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
@nellshamrell
nellshamrell force-pushed the table-driven-getauthorities-test branch from 8aec297 to 179e935 Compare July 27, 2026 21:57
@Hayden-IO
Hayden-IO requested a review from malancas July 27, 2026 23:22
@nellshamrell

Copy link
Copy Markdown
Contributor Author

Heads up: the failing CI on this PR is not related to these changes. It's a repo-wide breakage from an expired TUF root.json test fixture, tracked in #1999.

  • The failures are all in pkg/apis/policy/v1alpha1 (TestTrustRootValidation, TestReconcile) with tuf refresh failed: expired metadata error: final root.json is expired, plus the e2e jobs that depend on a healthy build.
  • This PR only touches pkg/apis/config/image_policies_test.go; that package's unit tests pass, along with gofmt, goimports, lint, DCO, whitespace, license, and codegen checks.
  • The same failures reproduce on main without this change.

This should go green once #1999 is resolved.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.73%. Comparing base (ead6e34) to head (9ec76b3).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1997   +/-   ##
=======================================
  Coverage   30.73%   30.73%           
=======================================
  Files         122      122           
  Lines        7409     7409           
=======================================
  Hits         2277     2277           
  Misses       4897     4897           
  Partials      235      235           

☔ View full report in Codecov by Harness.
📢 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.

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