Add contract tests for topological import ordering#16811
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- New pkg/parser/import_topological_contract_test.go with:
- verifyTopologicalConstraints: property-based helper that fails on
any topological violation without requiring an exact expected list
- TestImportTopologicalContractLexicalConflict: 4 fixtures where
lexical filename order conflicts with dependency order
- TestImportTopologicalContractTieBreakIsAlphabetical: documents and
verifies the alphabetical tie-break rule for independent nodes
- TestImportTopologicalContractCrossPath: validates the same fixture
via both the import processor and an independently-extracted dep map
- Modified pkg/cli/dependency_graph_test.go:
- TestDependencyGraph_TopologicalConsistencyContract: cross-path test
that runs DependencyGraph.BuildGraph and import processor on the same
fixtures and checks their topological constraints agree
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds contract-style tests to lock in deterministic topological import ordering (and the alphabetical tie-break rule), including cross-path consistency checks between the parser import processor and the CLI dependency graph.
Changes:
- Introduces
import_topological_contract_test.gowith property-based topological invariants + deterministic ordering fixtures. - Adds a CLI-level cross-path contract test asserting dependency-graph edges are respected by the import processor output order.
- Updates
action_pins.jsonby removing a couple of pin entries.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/workflow/data/action_pins.json | Removes two action pin entries/aliases. |
| pkg/parser/import_topological_contract_test.go | New contract tests for topological ordering, lexical-conflict fixtures, tie-break rule, and a cross-path verifier. |
| pkg/cli/dependency_graph_test.go | Adds a cross-path contract test comparing DependencyGraph edges to parser import ordering. |
Comments suppressed due to low confidence (1)
pkg/workflow/data/action_pins.json:166
- This file changes by removing action pin aliases (e.g. unversioned/short tags). The PR description doesn’t mention any action pin updates; if this is intentional, it should be called out explicitly (or moved to a separate PR) because it can affect workflows that reference the removed keys.
"github/codeql-action/upload-sarif@v3.32.3": {
"repo": "github/codeql-action/upload-sarif",
"version": "v3.32.3",
"sha": "17ec405481c24be464d4b2c5727a82f700ddb189"
},
"github/stale-repos@v3.0.2": {
"repo": "github/stale-repos",
"version": "v3.0.2",
"sha": "a21e55567b83cf3c3f3f9085d3038dc6cee02598"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for filename, content := range files { | ||
| fm, err := parser.ExtractFrontmatterFromContent(content) |
There was a problem hiding this comment.
The docstring says this function “reads each file in the given directory”, but it actually parses the provided files map and never reads from dir (which is currently unused). This makes the “independent dependency-map extracted from the fixture files” claim weaker than intended; consider either reading the written files from dir (os.ReadFile + ExtractFrontmatterFromContent) or updating the function name/signature/comments to reflect that it parses in-memory fixture contents.
| for filename, content := range files { | |
| fm, err := parser.ExtractFrontmatterFromContent(content) | |
| for filename := range files { | |
| fullPath := filepath.Join(dir, filename) | |
| contentBytes, err := os.ReadFile(fullPath) | |
| require.NoError(t, err, "reading fixture file %s", fullPath) | |
| fm, err := parser.ExtractFrontmatterFromContent(string(contentBytes)) |
| // indexOf returns the index of the first occurrence of sub in s, or -1 if not found. | ||
| func indexOf(s, sub string) int { | ||
| for i := 0; i <= len(s)-len(sub); i++ { | ||
| if s[i:i+len(sub)] == sub { | ||
| return i | ||
| } | ||
| } | ||
| return -1 |
There was a problem hiding this comment.
This reimplements substring search, but the standard library already provides strings.Index (and importing strings is already used elsewhere in the package). Using the standard helper would be clearer and avoids maintaining custom string-search logic.
| relImporter, err := filepath.Rel(workflowsDir, absImporter) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| importerIdx, ok := pos[relImporter] | ||
| if !ok { | ||
| continue // file not in the import processor result; skip | ||
| } | ||
| for _, absDep := range absDeps { | ||
| relDep, err := filepath.Rel(workflowsDir, absDep) | ||
| if err != nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
These filepath.Rel errors are silently ignored via continue. Since this is a contract test, it’s better to fail fast (e.g., t.Fatalf / require.NoError) so unexpected path issues can’t cause the test to pass without actually validating the contract.
Import ordering regressions (topological → lexical) are silent and hard to detect because happy-path compile tests don't lock in ordering semantics. This adds explicit contract tests that fail if ordering deviates from topological constraints, and documents the deterministic tie-break rule for independent nodes.
New:
pkg/parser/import_topological_contract_test.goverifyTopologicalConstraints— property-based helper asserting the core invariant: every dependency appears at a strictly lower index than its importer. Authoritative; independent of exact expected list.TestImportTopologicalContractLexicalConflict— fixtures where lexical filename order conflicts with dependency order (e.g.z-parent.mdimportsa-child.md). Any regression to lexical sorting breaks these tests.TestImportTopologicalContractTieBreakIsAlphabetical— documents and locks the tie-break rule: when multiple nodes are simultaneously ready (in-degree 0 in Kahn's algorithm), they are sorted alphabetically.TestImportTopologicalContractCrossPath— same fixture exercised via two independent paths: (1)ProcessImportsFromFrontmatterWithManifestfor the ordering, (2)buildDepMapFromFilesextracting dependencies directly from raw frontmatter. Ensures the ordering-producer and ordering-verifier agree.Modified:
pkg/cli/dependency_graph_test.goTestDependencyGraph_TopologicalConsistencyContract— CLI-level cross-path contract. Builds aDependencyGraphand runs the import processor on the same fixtures, then asserts every dependency edge captured by the graph is honoured in the processor'sImportedFilesoutput. Bridges the two code paths that both touch the same import graph.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw go 64/bin/go node /tmp�� /tmp/TestHashConsistency_WithImports3766729955/001/main.md git /usr/bin/git --show-toplevel go 64/bin/go git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v3/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha _.a GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env 1829-38053/test-3721526969/.github/workflows GO111MODULE k/gh-aw/gh-aw/actions/node_modules/.bin/node GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha ck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.pret--format=%H:%ct GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha ck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.prettierignore GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env 3 GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha user.email test@example.com /usr/bin/git --noprofile go 7f5643d5931eb119--show-toplevel git -C /tmp/shared-actions-test598975623 rev-parse /usr/bin/git re GO111MODULE 64/bin/go git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v5/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha 3685104295/.github/workflows GO111MODULE bin/sh GOINSECURE GOMOD GOMODCACHE go env */*.ts' '**/*.json' --ignore-path ../../../.pret.prettierignore GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD iles,SysoFiles,T--show-toplevel go env ub/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha prettier --check /opt/hostedtoolcache/go/1.25.0/x64/bin/go **/*.ts **/*.json --ignore-path go test�� -v -parallel=4 1/x64/bin/node -run=^Test ./... -short 1/x64/bin/node(http block)https://api.github.com/repos/actions/github-script/git/ref/tags/v8/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha npx prettier --wGOINSECURE node 64/bin/go ogical_contract_npm **/*.cjs 64/bin/go node /hom�� --write scripts/**/*.js /sh .prettierignore GO111MODULE run-script/lib/n"prettier" --check 'scripts/**/*.js' --ignore-path .prettierignore go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha "prettier" --wriGOINSECURE /opt/hostedtoolcGOMOD 64/bin/go ogical_contract_node -trimpath 64/bin/go node /hom�� ath ../../../.pr**/*.json ../../../**/*.js--ignore-path 64/bin/go --ignore-path ../../../.pretti-o 64/bin/go go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha "prettier" --wriGOINSECURE /opt/hostedtoolcGOMOD 64/bin/go n_pins.json -trimpath 64/bin/go go env ath ../../../.prettierignore go 64/bin/go --ignore-path ..git GO111MODULE 64/bin/go go(http block)https://api.github.com/repos/actions/setup-go/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha ck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.pret-n1 GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/actions/setup-node/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha ck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.pret-n1 GO111MODULE 1/x64/bin/node GOINSECURE GOMOD GOMODCACHE go -has�� ithub/workflows/blog-auditor.md GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts/usr/bin/gh gh run download 1 --dir test-logs/run-1 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run download 1 --dir test-logs/run-1 GO111MODULE ules/.bin/sh GOINSECURE GOMOD GOMODCACHE go env .js' --ignore-path .prettierignore GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts/usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE bin/node GOINSECURE GOMOD GOMODCACHE go tion�� -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts/usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link env -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE K0/Z2-ZXw9FhOpoporigin(http block)/usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE ules/.bin/node GOINSECURE GOMOD GOMODCACHE go tion�� -json GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts/usr/bin/gh gh run download 2 --dir test-logs/run-2 GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE ules/.bin/sh GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run download 2 --dir test-logs/run-2 GO111MODULE h GOINSECURE GOMOD GOMODCACHE go env .js' --ignore-path .prettierignore GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts/usr/bin/gh gh run download 3 --dir test-logs/run-3 GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE bin/sh GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run download 3 --dir test-logs/run-3 GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env .js' --ignore-path .prettierignore GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts/usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)/usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env .js' --ignore-path .prettierignore GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts/usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link env -json GO111MODULE h GOINSECURE GOMOD GOMODCACHE cx/AERBTSiwFBTdGDiIa9qx/H4W_iTHyWdoVr4iAs4gc(http block)/usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env .js' --ignore-path .prettierignore GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet(http block)https://api.github.com/repos/github/gh-aw/actions/workflows/usr/bin/gh gh workflow list --json name,state,path "prettier" --wriGOINSECURE sh 64/bin/go ath /dev/null GOPROXY 64/bin/go sh -c npx prettier --w**/*.ts go 64/bin/go -json GO111MODULE 64/bin/go go(http block)/usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 GO111MODULE x_amd64/cgo go /pre�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 GOMOD GOMODCACHE x_amd64/link env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE 2l/hKXTtrccq1d6W0qIfHov/iZTr9tjUBUCENUWdDWVs(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env e-analyzer.md GO111MODULE tions/setup/node_modules/.bin/node GOINSECURE GOMOD GOMODCACHE ns-lock.json does not exist yet"; \ fi(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha ath ../../../.pr**/*.json go 64/bin/go agent-persona-exinfocmp GO111MODULE cfg go er -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999/usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha y-test.md GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE F5/4qLQS-oeomrMmfGrEiad/6CmI5DOkrev-parse env -json GO111MODULE de_modules/.bin/node GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/nonexistent/repo/actions/runs/12345/usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE tions/setup/js/node_modules/.bin/sh GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE GOMOD GOMODCACHE go env ons/secrets GO111MODULE ache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/owner/repo/actions/workflows/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go ath /dev/null GOPROXY 64/bin/go node /opt�� prettier --write /prettier !../../../pkg/wogcc --ignore-path ../../../.pretti-x go(http block)/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go "prettier" --chenode GOPROXY 64/bin/go node /opt�� prettier --write /node !../../../pkg/wo/bin/sh --ignore-path ../../../.prettiecho "Syncing actions-lock.json from .github/aw to pkg/workflow/data/action_pinsremote.origin.url go(http block)/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go -json GO111MODULE 64/pkg/tool/linu-json git rev-�� --show-toplevel 64/pkg/tool/linuGOMOD 64/bin/go -json GO111MODULE ache/go/1.25.0/x--check git(http block)https://api.github.com/repos/owner/repo/contents/file.md/tmp/go-build599235328/b370/cli.test /tmp/go-build599235328/b370/cli.test -test.testlogfile=/tmp/go-build599235328/b370/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true /dev/null GOPROXY 64/bin/go sh -c "prettier" --wriGOINSECURE /opt/hostedtoolcGOMOD 64/bin/go ogical_contract_node -trimpath 64/bin/go node(http block)/tmp/go-build4010846226/b370/cli.test /tmp/go-build4010846226/b370/cli.test -test.testlogfile=/tmp/go-build4010846226/b370/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true */*.ts' '**/*.jsgo GO111MODULE 64/pkg/tool/linu-json git rev-�� --show-toplevel H5/B3jFDlCLvmLjTGOMOD 64/bin/go -json GO111MODULE ache/go/1.25.0/x"prettier" --check '**/*.cjs' '**/*.ts' '**/*.json' --ignore-path ../../../.prettierignore git(http block)https://api.github.com/repos/test-owner/test-repo/actions/secrets/usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name "prettier" --wriGOINSECURE sh 64/bin/go ath /dev/null GOPROXY 64/bin/go sh -c npx prettier --w**/*.ts go 64/bin/go -json GO111MODULE 64/bin/go go(http block)/usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name --show-toplevel go 64/bin/go -json GO111MODULE 64/pkg/tool/linu-json git rev-�� --show-toplevel 64/pkg/tool/linuGOMOD 64/bin/go -json GO111MODULE ache/go/1.25.0/x--check chmod(http block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.