From b3fdf3de38b096a890659058d4844de6804b3973 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:19:29 +0300 Subject: [PATCH 1/4] test(compilers/openapi): stop the reproducer table drifting The danglingDir comment said the directory holds twelve issue-#14 reproducers. It holds fourteen -- f31-discriminator-empty-name.yaml and f32-ref-noncanonical-escape.yaml postdate the number -- and the test's own table names all fourteen. A count in a comment beside the list it counts has no way to stay true, so drop it. Dropping the number leaves the drift it came from: the table is hand-maintained, so a fixture copied into the directory and not added to it is compiled by nothing while the suite stays green. Move the table into danglingReproducers() and hold it to the directory in both directions, which is what the comment now points at instead of a count. --- compilers/openapi/danglingcheck_test.go | 59 +++++++++++++++++++------ 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/compilers/openapi/danglingcheck_test.go b/compilers/openapi/danglingcheck_test.go index a144e460..37a49c58 100644 --- a/compilers/openapi/danglingcheck_test.go +++ b/compilers/openapi/danglingcheck_test.go @@ -22,8 +22,9 @@ import ( "github.com/dexpace/morphic/ir/irverify" ) -// danglingDir holds the twelve issue-#14 reproducers, copied out of triage so the -// tests are self-contained. +// danglingDir holds the issue-#14 reproducers, copied out of triage so the tests +// are self-contained. TestDanglingRefs_Reproducers' table is the enumeration, and +// TestDanglingRefs_EveryReproducerIsExercised holds it to the directory. const danglingDir = "../../testdata/dangling/openapi" // danglingRefs returns a sorted, human-readable list of every dangling reference @@ -83,16 +84,19 @@ const ( internsNoisy ) -// TestDanglingRefs_Reproducers compiles each issue-#14 reproducer and asserts the -// produced IR has zero dangling references — every offending entry either interns -// correctly or is dropped with an error-severity diagnostic. -func TestDanglingRefs_Reproducers(t *testing.T) { - t.Parallel() - cases := []struct { - file string - srcPath string - want outcome - }{ +// danglingCase is one reproducer: the fixture under danglingDir, the source path +// it is compiled as, and what the compiler is expected to do with it. +type danglingCase struct { + file string + srcPath string + want outcome +} + +// danglingReproducers enumerates the fixtures under danglingDir. +// TestDanglingRefs_EveryReproducerIsExercised fails when the directory holds one +// this list does not, so a fixture added to the corpus cannot sit unexercised. +func danglingReproducers() []danglingCase { + return []danglingCase{ {"f04-composition.yaml", "f04.yaml", drops}, {"f05-discriminator.yaml", "f05.yaml", drops}, {"f06-discriminator.yaml", "f06.yaml", drops}, @@ -108,7 +112,36 @@ func TestDanglingRefs_Reproducers(t *testing.T) { {"f31-discriminator-empty-name.yaml", "f31.yaml", interns}, {"f32-ref-noncanonical-escape.yaml", "f32.yaml", internsNoisy}, } - for _, tc := range cases { +} + +// TestDanglingRefs_EveryReproducerIsExercised holds danglingReproducers to the +// directory it enumerates. A fixture copied into danglingDir and not added to the +// table would otherwise be compiled by nothing, and the suite would stay green +// while the corpus grew past it — the drift a hand-maintained list invites. +func TestDanglingRefs_EveryReproducerIsExercised(t *testing.T) { + t.Parallel() + onDisk, err := filepath.Glob(filepath.Join(danglingDir, "*.yaml")) + require.NoError(t, err, "globbing the reproducer directory") + require.NotEmpty(t, onDisk, "the reproducer directory is not empty") + + listed := make(map[string]bool, len(danglingReproducers())) + for _, tc := range danglingReproducers() { + listed[tc.file] = true + } + for _, path := range onDisk { + assert.True(t, listed[filepath.Base(path)], + "%s is in %s but not in danglingReproducers", filepath.Base(path), danglingDir) + } + assert.Len(t, danglingReproducers(), len(onDisk), + "the table names a fixture the directory does not hold") +} + +// TestDanglingRefs_Reproducers compiles each issue-#14 reproducer and asserts the +// produced IR has zero dangling references — every offending entry either interns +// correctly or is dropped with an error-severity diagnostic. +func TestDanglingRefs_Reproducers(t *testing.T) { + t.Parallel() + for _, tc := range danglingReproducers() { t.Run(tc.file, func(t *testing.T) { t.Parallel() doc, diags := compileFile(t, danglingDir, tc.file, tc.srcPath) From b42c021ae4c4d07d093d2aa23dd6bac677fd55a3 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 15:56:18 +0300 Subject: [PATCH 2/4] test(compilers/openapi): hold the cycle tables to the corpus too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift this PR closes for the dangling reproducers has two more sites. cycleReproducers is a hand-maintained list of cycle_*.yaml fixtures, kept twice — once in this package for the compiler's refusal and once in scan for the pre-parse scan's diagnosis — and neither was held to the directory, so a fixture added to the corpus and left out of a table was exercised by nothing while the suite stayed green. Both tables now carry the twin of the dangling guard. Holding each to the same directory is also what keeps the two from drifting apart, which neither package can check directly since a test package cannot import another's. The path each reads fixtures from is spelled once as reproducerDir, so a guard and its reader cannot disagree about where they are looking. Beside them, two things in the guard this PR added: the length assertion catches a table row naming no fixture *and* fires beside the check above when the directory holds the extra, so its message no longer names one direction as though it were the only one; and the table is read once into a local rather than rebuilt for each of the three uses. --- compilers/openapi/cycles_test.go | 35 ++++++++++++++++++- compilers/openapi/danglingcheck_test.go | 12 ++++--- .../internal/scan/scan_internal_test.go | 34 +++++++++++++++++- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/compilers/openapi/cycles_test.go b/compilers/openapi/cycles_test.go index f28ebff4..24d74ae2 100644 --- a/compilers/openapi/cycles_test.go +++ b/compilers/openapi/cycles_test.go @@ -3,6 +3,7 @@ package openapi import ( "fmt" "os" + "path/filepath" "strings" "testing" @@ -53,6 +54,34 @@ var cycleReproducers = []struct{ name, file string }{ {"pointer-whitespace-self", "cycle_pointer_whitespace_self"}, } +// TestCycleReproducers_EveryFixtureIsExercised holds cycleReproducers to the +// fixtures on disk, the way TestDanglingRefs_EveryReproducerIsExercised holds +// its own table: a cycle_*.yaml added to the corpus and left out of the table is +// compiled by nothing here, and the suite stays green while the corpus grows +// past it. +// +// The scan package keeps a table of the same fixtures for its own assertion and +// carries the twin of this test. Holding both to one directory is what stops the +// two from drifting apart as well as from the corpus, which neither package can +// check directly — a test package cannot import another's. +func TestCycleReproducers_EveryFixtureIsExercised(t *testing.T) { + t.Parallel() + onDisk, err := filepath.Glob(filepath.Join(reproducerDir, "cycle_*.yaml")) + require.NoError(t, err, "globbing the reproducer corpus") + require.NotEmpty(t, onDisk, "the corpus holds cycle reproducers") + + listed := make(map[string]bool, len(cycleReproducers)) + for _, tc := range cycleReproducers { + listed[tc.file+".yaml"] = true + } + for _, path := range onDisk { + assert.True(t, listed[filepath.Base(path)], + "%s is in %s but not in cycleReproducers", filepath.Base(path), reproducerDir) + } + assert.Len(t, cycleReproducers, len(onDisk), + "cycleReproducers and %s hold different numbers of fixtures", reproducerDir) +} + func TestCompile_CyclicSpecDoesNotCrash(t *testing.T) { t.Parallel() for _, tc := range cycleReproducers { @@ -249,9 +278,13 @@ components: } } +// reproducerDir is where the cycle and amplification fixtures live, spelled once +// so the reader below and the table guard above cannot disagree about it. +const reproducerDir = "../../testdata/openapi" + func readReproducer(t *testing.T, file string) []byte { t.Helper() - data, err := os.ReadFile("../../testdata/openapi/" + file + ".yaml") + data, err := os.ReadFile(filepath.Join(reproducerDir, file+".yaml")) require.NoError(t, err) return data } diff --git a/compilers/openapi/danglingcheck_test.go b/compilers/openapi/danglingcheck_test.go index 37a49c58..01709b6e 100644 --- a/compilers/openapi/danglingcheck_test.go +++ b/compilers/openapi/danglingcheck_test.go @@ -124,16 +124,20 @@ func TestDanglingRefs_EveryReproducerIsExercised(t *testing.T) { require.NoError(t, err, "globbing the reproducer directory") require.NotEmpty(t, onDisk, "the reproducer directory is not empty") - listed := make(map[string]bool, len(danglingReproducers())) - for _, tc := range danglingReproducers() { + table := danglingReproducers() + listed := make(map[string]bool, len(table)) + for _, tc := range table { listed[tc.file] = true } for _, path := range onDisk { assert.True(t, listed[filepath.Base(path)], "%s is in %s but not in danglingReproducers", filepath.Base(path), danglingDir) } - assert.Len(t, danglingReproducers(), len(onDisk), - "the table names a fixture the directory does not hold") + // Neutral about which side is short, because it catches both: a row naming no + // fixture is what it is here for, and it fires again beside the check above + // when the directory is the side holding the extra. + assert.Len(t, table, len(onDisk), + "danglingReproducers and %s hold different numbers of fixtures", danglingDir) } // TestDanglingRefs_Reproducers compiles each issue-#14 reproducer and asserts the diff --git a/compilers/openapi/internal/scan/scan_internal_test.go b/compilers/openapi/internal/scan/scan_internal_test.go index cc033a40..13a77cf1 100644 --- a/compilers/openapi/internal/scan/scan_internal_test.go +++ b/compilers/openapi/internal/scan/scan_internal_test.go @@ -3,6 +3,7 @@ package scan import ( "fmt" "os" + "path/filepath" "strconv" "strings" "testing" @@ -53,6 +54,33 @@ var cycleReproducers = []struct{ name, file string }{ {"pointer-whitespace-self", "cycle_pointer_whitespace_self"}, } +// TestCycleReproducers_EveryFixtureIsExercised holds cycleReproducers to the +// fixtures on disk. A cycle_*.yaml added to the corpus and left out of the table +// is scanned by nothing here, and the suite stays green while the corpus grows +// past it. +// +// The compiler package keeps a table of the same fixtures for its own assertion +// and carries the twin of this test. Holding both to one directory is what stops +// the two from drifting apart as well as from the corpus, which neither package +// can check directly — a test package cannot import another's. +func TestCycleReproducers_EveryFixtureIsExercised(t *testing.T) { + t.Parallel() + onDisk, err := filepath.Glob(filepath.Join(reproducerDir, "cycle_*.yaml")) + require.NoError(t, err, "globbing the reproducer corpus") + require.NotEmpty(t, onDisk, "the corpus holds cycle reproducers") + + listed := make(map[string]bool, len(cycleReproducers)) + for _, tc := range cycleReproducers { + listed[tc.file+".yaml"] = true + } + for _, path := range onDisk { + assert.True(t, listed[filepath.Base(path)], + "%s is in %s but not in cycleReproducers", filepath.Base(path), reproducerDir) + } + assert.Len(t, cycleReproducers, len(onDisk), + "cycleReproducers and %s hold different numbers of fixtures", reproducerDir) +} + func TestDetectCycles_Reproducers(t *testing.T) { t.Parallel() for _, tc := range cycleReproducers { @@ -248,9 +276,13 @@ func rawNodes(n *yaml.Node) int64 { return sourceindex.Build(n, sourceindex.MaxIndexedNodes).Nodes() } +// reproducerDir is where the cycle fixtures live, spelled once so the reader +// below and the table guard cannot disagree about it. +const reproducerDir = "../../../../testdata/openapi" + func readReproducer(t *testing.T, file string) []byte { t.Helper() - data, err := os.ReadFile("../../../../testdata/openapi/" + file + ".yaml") + data, err := os.ReadFile(filepath.Join(reproducerDir, file+".yaml")) require.NoError(t, err) return data } From 36b17cc55cfd83e714ee4a1806f31aa4c1a98a00 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 16:12:58 +0300 Subject: [PATCH 3/4] test(compilers/openapi): derive the fuzz seed paths from reproducerDir The constant introduced with the table guard said the fixture directory was spelled once, and it was not: the fuzz seed loader in the same file still spelled it twice, so the guard and the seeds could have ended up looking in different places. They matter differently. readReproducer requires the read to succeed, so a wrong path there fails loudly; the seed loader ignores the error and adds nothing, so a wrong path there costs the fuzzer its seeds in silence. Both derive from the constant now, and the bomb fixture uses the constant the package already keeps for it. Seed count is unchanged at 36. --- compilers/openapi/cycles_test.go | 11 +++++++---- compilers/openapi/internal/scan/scan_internal_test.go | 5 +++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/compilers/openapi/cycles_test.go b/compilers/openapi/cycles_test.go index 24d74ae2..0446436f 100644 --- a/compilers/openapi/cycles_test.go +++ b/compilers/openapi/cycles_test.go @@ -278,8 +278,11 @@ components: } } -// reproducerDir is where the cycle and amplification fixtures live, spelled once -// so the reader below and the table guard above cannot disagree about it. +// reproducerDir is where the cycle fixtures live. The reader below, the table +// guard above and the fuzz seeds all derive their paths from it, so none of them +// can end up looking somewhere the others are not — which matters most for the +// seed loader, since it ignores a read error and a wrong path there would cost +// the fuzzer its seeds in silence. const reproducerDir = "../../testdata/openapi" func readReproducer(t *testing.T, file string) []byte { @@ -318,14 +321,14 @@ func TestCompile_MergeChainPastBoundStillCompiles(t *testing.T) { func FuzzCycleDetector(f *testing.F) { for _, tc := range cycleReproducers { - if data, err := os.ReadFile("../../testdata/openapi/" + tc.file + ".yaml"); err == nil { + if data, err := os.ReadFile(filepath.Join(reproducerDir, tc.file+".yaml")); err == nil { f.Add(data) } } for _, tc := range refShapedDataSpecs { f.Add([]byte(tc.data)) } - if data, err := os.ReadFile("../../testdata/openapi/amplification_alias_bomb.yaml"); err == nil { + if data, err := os.ReadFile(amplificationBombFixture); err == nil { f.Add(data) // the GitHub #27 reproducer: refused for amplification, not a cycle } f.Add([]byte(" ")) // whitespace-only: recoverable parser panic diff --git a/compilers/openapi/internal/scan/scan_internal_test.go b/compilers/openapi/internal/scan/scan_internal_test.go index 13a77cf1..d14dc307 100644 --- a/compilers/openapi/internal/scan/scan_internal_test.go +++ b/compilers/openapi/internal/scan/scan_internal_test.go @@ -276,8 +276,9 @@ func rawNodes(n *yaml.Node) int64 { return sourceindex.Build(n, sourceindex.MaxIndexedNodes).Nodes() } -// reproducerDir is where the cycle fixtures live, spelled once so the reader -// below and the table guard cannot disagree about it. +// reproducerDir is where the cycle fixtures live. The reader below and the table +// guard both derive their paths from it, so neither can end up looking somewhere +// the other is not. const reproducerDir = "../../../../testdata/openapi" func readReproducer(t *testing.T, file string) []byte { From 9c6e35e9748e91462e276e489c3483ad25296b6e Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 16:26:46 +0300 Subject: [PATCH 4/4] docs(compilers/openapi): name the table where the extraction left it The danglingDir comment credited the enumeration to TestDanglingRefs_Reproducers' table, which is where it was before this branch moved it into danglingReproducers and left the comment behind. --- compilers/openapi/danglingcheck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compilers/openapi/danglingcheck_test.go b/compilers/openapi/danglingcheck_test.go index 01709b6e..66c83e80 100644 --- a/compilers/openapi/danglingcheck_test.go +++ b/compilers/openapi/danglingcheck_test.go @@ -23,7 +23,7 @@ import ( ) // danglingDir holds the issue-#14 reproducers, copied out of triage so the tests -// are self-contained. TestDanglingRefs_Reproducers' table is the enumeration, and +// are self-contained. danglingReproducers is the enumeration, and // TestDanglingRefs_EveryReproducerIsExercised holds it to the directory. const danglingDir = "../../testdata/dangling/openapi"