Skip to content

Commit 23ab38d

Browse files
[purelock] Lock down parseImportSpecsFromObject, relativizeIncludedFilePath, resolveCacheStepName with pure-function test suites (#54235)
1 parent a5f0dd3 commit 23ab38d

4 files changed

Lines changed: 313 additions & 0 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# ADR-54235: Adopt PureLock Batch Testing for Pure-Function Coverage Lockdown
2+
3+
**Date**: 2026-08-20
4+
**Status**: Draft
5+
**Deciders**: pelikhan, PureLock automation (app/github-actions)
6+
7+
---
8+
9+
### Context
10+
11+
Three Go functions in `pkg/parser` and `pkg/workflow` had 0% function coverage with no systematic mechanism to detect or address such gaps: `parseImportSpecsFromObject` (`pkg/parser/import_bfs.go:88`), `relativizeIncludedFilePath` (`pkg/parser/include_expander.go:100`), and `resolveCacheStepName` (`pkg/workflow/cache_steps.go:75`). All three are pure functions — no I/O, no global mutation, no observable side effects — making them safe targets for automated test generation. The PureLock workflow identifies pure-function coverage gaps from a ranked candidate list and generates maximal-coverage table-driven test suites in batch PRs to close those gaps.
12+
13+
### Decision
14+
15+
We will use the PureLock automated batch workflow to systematically identify pure Go functions with 0% function coverage and lock them down with table-driven unit test suites targeting 100% function coverage per function. Each batch PR targets functions confirmed pure by static analysis and manual inspection; fuzz testing is used only when marked explicitly fuzz-friendly and table-driven cases do not achieve full coverage. Tests are committed directly to the package under test (e.g., `pkg/parser/import_bfs_test.go`) using the `!integration` build tag.
16+
17+
### Alternatives Considered
18+
19+
#### Alternative 1: Rely on ad-hoc test coverage during feature work
20+
21+
The default model where developers add tests for new or changed code as part of feature PRs. This is well-understood and keeps tests co-located with the motivating change, but it does not systematically surface coverage gaps in pre-existing pure functions that have never been tested. The three functions in this PR had been in the codebase with 0% coverage indefinitely.
22+
23+
#### Alternative 2: Use Go fuzzing (`go test -fuzz`) for pure-function coverage
24+
25+
Some PureLock candidates are marked fuzz-friendly. Fuzzing is powerful for detecting edge cases in deterministic functions, but it requires a persistent corpus, is slower than table-driven tests in CI, and is overkill when a small set of structural branches can be exhaustively enumerated. For these three functions, table-driven cases provided 100% function coverage without fuzzing overhead.
26+
27+
### Consequences
28+
29+
#### Positive
30+
- `parseImportSpecsFromObject`, `relativizeIncludedFilePath`, and `resolveCacheStepName` move from 0% to 100% function coverage, protecting against regressions in pure business logic.
31+
- Establishes a repeatable, auditable batch pattern for closing coverage gaps in side-effect-free functions across the codebase.
32+
- Test suites are fully deterministic and run under `-race`, catching data-race regressions.
33+
34+
#### Negative
35+
- Automated test-only batch PRs cross volume thresholds in business logic directories, triggering ADR enforcement gates even when no production architecture is changing.
36+
- Reviewers must validate AI-generated test cases match the intended behavior of the production function, not just the observed behavior at generation time.
37+
- Pre-existing unrelated test failures in the sandbox environment (network-restricted and `/dev/fd`-dependent tests) must be manually distinguished from failures introduced by this change.
38+
39+
#### Neutral
40+
- Package-level statement coverage changes are marginal (e.g., `pkg/parser` 72.1% → 72.4%) because function coverage targets only the specific locked-down functions, not untested statement branches within other functions.
41+
- The `!integration` build tag keeps these tests out of integration test runs, consistent with existing patterns in `pkg/parser` and `pkg/workflow`.
42+
43+
---
44+
45+
*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/parser/import_bfs_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,90 @@ func TestParseImportSpecsFromArray_RejectsIfField(t *testing.T) {
1818
require.Error(t, err)
1919
require.ErrorContains(t, err, "import 'if' is no longer supported")
2020
}
21+
22+
func TestParseImportSpecsFromObject(t *testing.T) {
23+
tests := []struct {
24+
name string
25+
importsObject map[string]any
26+
wantSpecs []ImportSpec
27+
wantErr string
28+
}{
29+
{
30+
name: "no aw key returns nil, nil",
31+
importsObject: map[string]any{},
32+
wantSpecs: nil,
33+
},
34+
{
35+
name: "aw key present but nil value falls to default case",
36+
importsObject: map[string]any{"aw": nil},
37+
wantErr: "imports.aw must be an array of strings or objects",
38+
},
39+
{
40+
name: "aw as []any of strings",
41+
importsObject: map[string]any{
42+
"aw": []any{"shared/a.md", "shared/b.md"},
43+
},
44+
wantSpecs: []ImportSpec{{Path: "shared/a.md"}, {Path: "shared/b.md"}},
45+
},
46+
{
47+
name: "aw as []any of objects",
48+
importsObject: map[string]any{
49+
"aw": []any{
50+
map[string]any{"path": "shared/c.md"},
51+
},
52+
},
53+
wantSpecs: []ImportSpec{{Path: "shared/c.md"}},
54+
},
55+
{
56+
name: "aw as []any propagates array parse error",
57+
importsObject: map[string]any{
58+
"aw": []any{
59+
map[string]any{"nope": "shared/d.md"},
60+
},
61+
},
62+
wantErr: "imports.aw: import object must have a 'path' or 'uses' field",
63+
},
64+
{
65+
name: "aw as []string",
66+
importsObject: map[string]any{
67+
"aw": []string{"shared/e.md", "shared/f.md"},
68+
},
69+
wantSpecs: []ImportSpec{{Path: "shared/e.md"}, {Path: "shared/f.md"}},
70+
},
71+
{
72+
name: "aw as []string empty slice returns empty (non-nil) specs",
73+
importsObject: map[string]any{
74+
"aw": []string{},
75+
},
76+
wantSpecs: []ImportSpec{},
77+
},
78+
{
79+
name: "aw as unsupported type (string) returns error",
80+
importsObject: map[string]any{
81+
"aw": "shared/g.md",
82+
},
83+
wantErr: "imports.aw must be an array of strings or objects",
84+
},
85+
{
86+
name: "aw as unsupported type (map) returns error",
87+
importsObject: map[string]any{
88+
"aw": map[string]any{"path": "shared/h.md"},
89+
},
90+
wantErr: "imports.aw must be an array of strings or objects",
91+
},
92+
}
93+
94+
for _, tt := range tests {
95+
t.Run(tt.name, func(t *testing.T) {
96+
specs, err := parseImportSpecsFromObject(tt.importsObject)
97+
if tt.wantErr != "" {
98+
require.Error(t, err)
99+
require.ErrorContains(t, err, tt.wantErr)
100+
require.Nil(t, specs)
101+
return
102+
}
103+
require.NoError(t, err)
104+
require.Equal(t, tt.wantSpecs, specs)
105+
})
106+
}
107+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
//go:build !integration
2+
3+
package parser
4+
5+
import (
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestRelativizeIncludedFilePath(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
baseDir string
16+
repoRoot string
17+
filePath string
18+
want string
19+
}{
20+
{
21+
name: "file under baseDir returns baseDir-relative slash path",
22+
baseDir: filepath.FromSlash("/repo/workflows"),
23+
repoRoot: filepath.FromSlash("/repo"),
24+
filePath: filepath.FromSlash("/repo/workflows/shared/a.md"),
25+
want: "shared/a.md",
26+
},
27+
{
28+
name: "file equal to baseDir returns dot",
29+
baseDir: filepath.FromSlash("/repo/workflows"),
30+
repoRoot: filepath.FromSlash("/repo"),
31+
filePath: filepath.FromSlash("/repo/workflows"),
32+
want: ".",
33+
},
34+
{
35+
name: "file outside baseDir but under repoRoot returns repoRoot-relative slash path",
36+
baseDir: filepath.FromSlash("/repo/workflows"),
37+
repoRoot: filepath.FromSlash("/repo"),
38+
filePath: filepath.FromSlash("/repo/.github/shared/b.md"),
39+
want: ".github/shared/b.md",
40+
},
41+
{
42+
name: "file outside both baseDir and repoRoot returns slash-converted absolute path",
43+
baseDir: filepath.FromSlash("/repo/workflows"),
44+
repoRoot: filepath.FromSlash("/repo"),
45+
filePath: filepath.FromSlash("/other/shared/c.md"),
46+
want: "/other/shared/c.md",
47+
},
48+
{
49+
name: "empty repoRoot with file outside baseDir falls back to slash-converted path",
50+
baseDir: filepath.FromSlash("/repo/workflows"),
51+
repoRoot: "",
52+
filePath: filepath.FromSlash("/other/shared/d.md"),
53+
want: "/other/shared/d.md",
54+
},
55+
{
56+
name: "empty repoRoot with file under baseDir still resolves via baseDir",
57+
baseDir: filepath.FromSlash("/repo/workflows"),
58+
repoRoot: "",
59+
filePath: filepath.FromSlash("/repo/workflows/e.md"),
60+
want: "e.md",
61+
},
62+
{
63+
name: "file equal to repoRoot when outside baseDir returns dot",
64+
baseDir: filepath.FromSlash("/repo/workflows"),
65+
repoRoot: filepath.FromSlash("/repo"),
66+
filePath: filepath.FromSlash("/repo"),
67+
want: ".",
68+
},
69+
}
70+
71+
for _, tt := range tests {
72+
t.Run(tt.name, func(t *testing.T) {
73+
got := relativizeIncludedFilePath(tt.baseDir, tt.repoRoot, tt.filePath)
74+
require.Equal(t, tt.want, got)
75+
})
76+
}
77+
}

pkg/workflow/cache_steps_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
//go:build !integration
2+
3+
package workflow
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestResolveCacheStepName(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
cache map[string]any
15+
idx int
16+
total int
17+
want string
18+
}{
19+
{
20+
name: "name field present takes priority over everything",
21+
cache: map[string]any{"name": "My Cache", "key": "some-key"},
22+
idx: 0,
23+
total: 1,
24+
want: "My Cache",
25+
},
26+
{
27+
name: "empty name string falls through to key",
28+
cache: map[string]any{"name": "", "key": "some-key"},
29+
idx: 0,
30+
total: 1,
31+
want: "Cache (some-key)",
32+
},
33+
{
34+
name: "non-string name falls through to key",
35+
cache: map[string]any{"name": 42, "key": "some-key"},
36+
idx: 0,
37+
total: 1,
38+
want: "Cache (some-key)",
39+
},
40+
{
41+
name: "no name, key present, single total returns key-based name",
42+
cache: map[string]any{"key": "npm-cache"},
43+
idx: 0,
44+
total: 1,
45+
want: "Cache (npm-cache)",
46+
},
47+
{
48+
name: "empty key string falls through to default stepName",
49+
cache: map[string]any{"key": ""},
50+
idx: 0,
51+
total: 1,
52+
want: "Cache",
53+
},
54+
{
55+
name: "non-string key falls through to default stepName",
56+
cache: map[string]any{"key": 123},
57+
idx: 0,
58+
total: 1,
59+
want: "Cache",
60+
},
61+
{
62+
name: "no name or key, single total returns default Cache",
63+
cache: map[string]any{},
64+
idx: 0,
65+
total: 1,
66+
want: "Cache",
67+
},
68+
{
69+
name: "no name or key, multiple total returns indexed default",
70+
cache: map[string]any{},
71+
idx: 0,
72+
total: 3,
73+
want: "Cache 1",
74+
},
75+
{
76+
name: "no name or key, multiple total uses 1-based idx",
77+
cache: map[string]any{},
78+
idx: 2,
79+
total: 3,
80+
want: "Cache 3",
81+
},
82+
{
83+
name: "key present with multiple total still prefers key over indexed default",
84+
cache: map[string]any{"key": "build-cache"},
85+
idx: 1,
86+
total: 2,
87+
want: "Cache (build-cache)",
88+
},
89+
{
90+
name: "name present with multiple total still prefers name",
91+
cache: map[string]any{"name": "Custom"},
92+
idx: 1,
93+
total: 2,
94+
want: "Custom",
95+
},
96+
}
97+
98+
for _, tt := range tests {
99+
t.Run(tt.name, func(t *testing.T) {
100+
got := resolveCacheStepName(tt.cache, tt.idx, tt.total)
101+
require.Equal(t, tt.want, got)
102+
})
103+
}
104+
}

0 commit comments

Comments
 (0)