From eeb4a42a83560f469960d212cc53709640ac1ffe Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 15 Oct 2026 12:55:09 +0900 Subject: [PATCH 1/2] feat: let the record store keep the last changed-files check of an assembly The store can now record what a check of an assembly's changed files reported, under the input key, the baseline and the worker build it checked with, and hand it back only while all three are the same. A record is never written without all three keys, is written through a temporary file, and Forget removes it with the assembly's other records. Nothing writes it yet. --- .../compilecheck/resultstore/fast_check.go | 121 +++++++++ .../resultstore/fast_check_test.go | 233 ++++++++++++++++++ .../compilecheck/resultstore/store.go | 18 +- 3 files changed, 368 insertions(+), 4 deletions(-) create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/fast_check.go create mode 100644 cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go diff --git a/cli/dispatcher/internal/compilecheck/resultstore/fast_check.go b/cli/dispatcher/internal/compilecheck/resultstore/fast_check.go new file mode 100644 index 000000000..e31bed27b --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/fast_check.go @@ -0,0 +1,121 @@ +package resultstore + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +const ( + // fastResultFormatVersion guards against replaying a check an older or newer build recorded: what + // the record holds is part of this check's code. + fastResultFormatVersion = 1 + + fastResultExtension = ".fastresult.json" +) + +// FastCheckResult is what one check of an assembly's changed files reported. +type FastCheckResult[D any] struct { + Succeeded bool + Diagnostics []D + ChangedFiles []string + UncheckedCodes []string + GeneratorsNotRerun bool +} + +// fastCheckRecord is what one check of an assembly's changed files left behind for the next run: the +// inputs, the baseline and the worker build it checked with, and what it reported. +type fastCheckRecord[D any] struct { + FormatVersion int + InputKey string + BaselineInputKey string + WorkerKey string + Succeeded bool + Diagnostics []D + ChangedFiles []string + UncheckedCodes []string + GeneratorsNotRerun bool +} + +// fastResultPath names where one assembly's recorded check lives. +func fastResultPath(directoryPath string, assembly string) string { + return filepath.Join(directoryPath, assembly+fastResultExtension) +} + +// ReusableFastCheck reports the recorded check of the assembly when it answered for these inputs +// (key), against this baseline, with this worker build. Anything unexpected means no. +// Why anything unexpected simply means no: the caller checks the changed files again, which is what +// it would have done anyway. +func (s Store[D]) ReusableFastCheck( + assembly string, key string, baseline Baseline[D], workerKey string, +) (FastCheckResult[D], bool) { + if key == "" { + return FastCheckResult[D]{}, false + } + content, err := os.ReadFile(fastResultPath(s.directoryPath(), assembly)) + if err != nil { + return FastCheckResult[D]{}, false + } + + var record fastCheckRecord[D] + if err := json.Unmarshal(content, &record); err != nil { + return FastCheckResult[D]{}, false + } + if record.FormatVersion != fastResultFormatVersion || record.InputKey != key || + record.BaselineInputKey != baseline.InputKey || record.WorkerKey != workerKey { + return FastCheckResult[D]{}, false + } + + return FastCheckResult[D]{ + Succeeded: record.Succeeded, + Diagnostics: record.Diagnostics, + ChangedFiles: record.ChangedFiles, + UncheckedCodes: record.UncheckedCodes, + GeneratorsNotRerun: record.GeneratorsNotRerun, + }, true +} + +// RecordFastCheck records what a check of the assembly's changed files reported for these inputs, +// against this baseline, with this worker build, through a temporary file. +// Why a missing key is refused rather than recorded: a record under an empty key would later match +// another empty key, and replay a check of inputs nobody can name. +func (s Store[D]) RecordFastCheck( + assembly string, key string, baseline Baseline[D], workerKey string, result FastCheckResult[D], +) error { + if missing := missingFastCheckKey(key, baseline.InputKey, workerKey); missing != "" { + return fmt.Errorf("refusing to record a changed-files check of %s without %s", assembly, missing) + } + + content, err := json.Marshal(fastCheckRecord[D]{ + FormatVersion: fastResultFormatVersion, + InputKey: key, + BaselineInputKey: baseline.InputKey, + WorkerKey: workerKey, + Succeeded: result.Succeeded, + Diagnostics: result.Diagnostics, + ChangedFiles: result.ChangedFiles, + UncheckedCodes: result.UncheckedCodes, + GeneratorsNotRerun: result.GeneratorsNotRerun, + }) + if err != nil { + return fmt.Errorf("failed to record the changed-files check of %s: %w", assembly, err) + } + + return writeFileAtomically(s.directoryPath(), fastResultPath(s.directoryPath(), assembly), content) +} + +// missingFastCheckKey names the first of the keys a check is looked up by that is empty, or returns +// "" when all three are there. +func missingFastCheckKey(key string, baselineInputKey string, workerKey string) string { + switch { + case key == "": + return "an input key" + case baselineInputKey == "": + return "a baseline" + case workerKey == "": + return "a worker build" + default: + return "" + } +} diff --git a/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go b/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go new file mode 100644 index 000000000..ddef9264a --- /dev/null +++ b/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go @@ -0,0 +1,233 @@ +package resultstore + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// checkedBaseline is the baseline the tests' checks ran against. +func checkedBaseline() Baseline[testDiagnostic] { + return Baseline[testDiagnostic]{InputKey: "baseline"} +} + +// checkedResult is a check the tests record, with every field set so a field the store drops shows. +func checkedResult() FastCheckResult[testDiagnostic] { + return FastCheckResult[testDiagnostic]{ + Succeeded: true, + Diagnostics: []testDiagnostic{{ + Severity: "warning", Code: "CS0168", Message: "unused", File: "Assets/Alpha/Alpha.cs", Line: 3, Column: 9, + }}, + ChangedFiles: []string{"Assets/Alpha/Alpha.cs"}, + UncheckedCodes: []string{"CS0169"}, + GeneratorsNotRerun: true, + } +} + +// recordCheck records checkedResult for Alpha under the input key "input", the checked baseline and +// the worker build "worker". +func recordCheck(t *testing.T, directoryPath string) { + t.Helper() + err := directoryStore(directoryPath).RecordFastCheck("Alpha", "input", checkedBaseline(), "worker", checkedResult()) + if err != nil { + t.Fatalf("failed to record the check: %v", err) + } +} + +// rewriteCheckRecord reads Alpha's recorded check, lets change alter it, and writes it back. +func rewriteCheckRecord(t *testing.T, directoryPath string, change func(record *fastCheckRecord[testDiagnostic])) { + t.Helper() + path := fastResultPath(directoryPath, "Alpha") + content, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read the recorded check: %v", err) + } + var record fastCheckRecord[testDiagnostic] + if err := json.Unmarshal(content, &record); err != nil { + t.Fatalf("failed to parse the recorded check: %v", err) + } + change(&record) + rewritten, err := json.Marshal(record) + if err != nil { + t.Fatalf("failed to encode the recorded check: %v", err) + } + if err := os.WriteFile(path, rewritten, 0o600); err != nil { + t.Fatalf("failed to write the recorded check back: %v", err) + } +} + +// Verifies a recorded check comes back exactly as it was recorded. +func TestReusableFastCheckReturnsTheRecordedCheck(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + + result, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "worker") + + if !found { + t.Fatal("expected the recorded check to be found") + } + if !reflect.DeepEqual(result, checkedResult()) { + t.Errorf("result = %+v, want %+v", result, checkedResult()) + } +} + +// Verifies a check recorded for other inputs is not handed out. +func TestReusableFastCheckRejectsAnotherInputKey(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "other", checkedBaseline(), "worker"); found { + t.Error("a check recorded for other inputs should not be found") + } +} + +// Verifies a check recorded against another baseline is not handed out. +func TestReusableFastCheckRejectsAnotherBaseline(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + baseline := Baseline[testDiagnostic]{InputKey: "other baseline"} + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", baseline, "worker"); found { + t.Error("a check recorded against another baseline should not be found") + } +} + +// Verifies a check recorded by another worker build is not handed out. +func TestReusableFastCheckRejectsAnotherWorkerBuild(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "other"); found { + t.Error("a check recorded by another worker build should not be found") + } +} + +// Verifies a check recorded by another version of compile-check is not handed out. +func TestReusableFastCheckRejectsAnotherFormatVersion(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + rewriteCheckRecord(t, directoryPath, func(record *fastCheckRecord[testDiagnostic]) { + record.FormatVersion = fastResultFormatVersion - 1 + }) + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "worker"); found { + t.Error("a check of another format version should not be found") + } +} + +// Verifies a record that cannot be read is treated as no record. +func TestReusableFastCheckRejectsAnUnreadableRecord(t *testing.T) { + directoryPath := t.TempDir() + writeOutputFile(t, directoryPath, "Alpha"+fastResultExtension, "{not json") + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "worker"); found { + t.Error("an unreadable record should not be found") + } +} + +// Verifies an empty input key never matches, even a record that holds an empty input key. +func TestReusableFastCheckRejectsAnEmptyInputKey(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + rewriteCheckRecord(t, directoryPath, func(record *fastCheckRecord[testDiagnostic]) { + record.InputKey = "" + }) + + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "", checkedBaseline(), "worker"); found { + t.Error("an empty input key should not find a record") + } +} + +// Verifies a check is not recorded without each of the three keys it is looked up by, and nothing is +// written in that case. +func TestRecordFastCheckRefusesAMissingKey(t *testing.T) { + for name, testCase := range map[string]struct { + key string + baseline Baseline[testDiagnostic] + workerKey string + want string + }{ + "input key": {key: "", baseline: checkedBaseline(), workerKey: "worker", want: "an input key"}, + "baseline": {key: "input", baseline: Baseline[testDiagnostic]{}, workerKey: "worker", want: "a baseline"}, + "worker": {key: "input", baseline: checkedBaseline(), workerKey: "", want: "a worker build"}, + } { + t.Run(name, func(t *testing.T) { + directoryPath := t.TempDir() + + err := directoryStore(directoryPath).RecordFastCheck( + "Alpha", testCase.key, testCase.baseline, testCase.workerKey, checkedResult()) + + want := "refusing to record a changed-files check of Alpha without " + testCase.want + if err == nil || err.Error() != want { + t.Errorf("err = %v, want %q", err, want) + } + if _, statErr := os.Stat(fastResultPath(directoryPath, "Alpha")); !os.IsNotExist(statErr) { + t.Errorf("expected no record to be written, got %v", statErr) + } + }) + } +} + +// Verifies a check that cannot be renamed into place is reported as a write failure and leaves no +// temporary file behind. +func TestRecordFastCheckReportsAFailedRename(t *testing.T) { + directoryPath := t.TempDir() + blockManifestPath(t, fastResultPath(directoryPath, "Alpha")) + + err := directoryStore(directoryPath).RecordFastCheck("Alpha", "input", checkedBaseline(), "worker", checkedResult()) + + if err == nil || !strings.HasPrefix(err.Error(), "failed to write "+fastResultPath(directoryPath, "Alpha")+":") { + t.Fatalf("err = %v", err) + } + matches, globErr := filepath.Glob(filepath.Join(directoryPath, "*"+resultManifestTempSuffix)) + if globErr != nil || len(matches) != 0 { + t.Fatalf("temporary records left behind: %v (glob error %v)", matches, globErr) + } +} + +// Verifies forgetting an assembly removes its recorded check too, so no check outlives the compile +// it was checked against. +func TestForgetRemovesTheRecordedCheck(t *testing.T) { + directoryPath := t.TempDir() + recordCheck(t, directoryPath) + + if err := directoryStore(directoryPath).Forget("Alpha"); err != nil { + t.Fatalf("expected the records to be removed, got error: %v", err) + } + + if _, err := os.Stat(fastResultPath(directoryPath, "Alpha")); !os.IsNotExist(err) { + t.Errorf("expected the recorded check to be removed, got %v", err) + } +} + +// Verifies the bytes a check is recorded as, so the record keeps its format. +func TestRecordFastCheckWritesTheRecordedFormat(t *testing.T) { + directoryPath := t.TempDir() + result := FastCheckResult[testDiagnostic]{ + Succeeded: false, + Diagnostics: []testDiagnostic{{ + Severity: "error", + Code: "CS0103", + Message: "The name 'Missing' does not exist in the current context", + File: "Assets/Alpha/Alpha.cs", + Line: 3, + Column: 9, + }}, + ChangedFiles: []string{"Assets/Alpha/Alpha.cs"}, + UncheckedCodes: []string{"CS0169"}, + GeneratorsNotRerun: true, + } + + if err := directoryStore(directoryPath).RecordFastCheck("Alpha", "input", checkedBaseline(), "worker", result); err != nil { + t.Fatalf("failed to record the check: %v", err) + } + + assertFileContent(t, fastResultPath(directoryPath, "Alpha"), `{"FormatVersion":1,"InputKey":"input",`+ + `"BaselineInputKey":"baseline","WorkerKey":"worker","Succeeded":false,`+ + `"Diagnostics":[{"Severity":"error","Code":"CS0103","Message":"The name 'Missing' does not exist in the current context",`+ + `"File":"Assets/Alpha/Alpha.cs","Line":3,"Column":9}],`+ + `"ChangedFiles":["Assets/Alpha/Alpha.cs"],"UncheckedCodes":["CS0169"],"GeneratorsNotRerun":true}`) +} diff --git a/cli/dispatcher/internal/compilecheck/resultstore/store.go b/cli/dispatcher/internal/compilecheck/resultstore/store.go index 929d4dc22..ab0b17bdc 100644 --- a/cli/dispatcher/internal/compilecheck/resultstore/store.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/store.go @@ -1,12 +1,14 @@ // Package resultstore keeps what earlier runs of compile-check recorded for each assembly in one // output directory, and is the only code that reads or writes those records. // -// A store keeps three records per assembly: +// A store keeps four records per assembly: // - the result of its last compile: the inputs it ran on (as a key), what it reported, and a hash // of every output it wrote; // - the baseline of the changed-files check: what the last successful full compile saw of each // source, its non-source inputs and the worker build that fingerprinted it; -// - the generator output that full compile saved. +// - the generator output that full compile saved; +// - the result of its last changed-files check: the inputs, the baseline and the worker build it +// checked with, and what it reported. // // Contract: // - Records are written through a temporary file and renamed into place, so a run stopped while @@ -21,6 +23,8 @@ // "no baseline: the last full compile did not record one", // "no baseline: it was recorded by another version of compile-check", // "baseline compile is no longer on disk". +// - A changed-files check is reusable only under the input key, the baseline and the worker build +// it was recorded with, and is never recorded without all three. // - Forget removes every kind of record of an assembly. It does not touch the assembly's outputs, // and the store never creates the generator output directory; it only names where it is. package resultstore @@ -54,8 +58,8 @@ func (s Store[D]) directoryPath() string { return filepath.Join(s.projectRoot, s.outputDir) } -// Forget removes every record of the assembly: its compile result, its baseline, and its saved -// generator output. +// Forget removes every record of the assembly: its compile result, its baseline, its saved generator +// output, and its last changed-files check. func (s Store[D]) Forget(assembly string) error { names := []string{ // Why the compile result goes with the outputs: it describes those very files, so leaving it @@ -76,6 +80,12 @@ func (s Store[D]) Forget(assembly string) error { return fmt.Errorf("failed to remove %s: %w", generatedPath, err) } + // The last changed-files check was checked against the compile whose records are going. + fastResult := fastResultPath(s.directoryPath(), assembly) + if err := os.Remove(fastResult); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove %s: %w", fastResult, err) + } + return nil } From 9b5bd6b14b955883c9b6adad2f8a539289eae131 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 15 Oct 2026 13:04:33 +0900 Subject: [PATCH 2/2] test: reject a recorded check with a field of the wrong type A record that is not JSON decodes to nothing, so it was rejected by its format version even when the decode error was ignored. A record whose version and keys match but whose one field has the wrong type decodes partly, and only the decode error stops it from being handed out. --- .../resultstore/fast_check_test.go | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go b/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go index ddef9264a..08855665a 100644 --- a/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go +++ b/cli/dispatcher/internal/compilecheck/resultstore/fast_check_test.go @@ -118,13 +118,35 @@ func TestReusableFastCheckRejectsAnotherFormatVersion(t *testing.T) { } } -// Verifies a record that cannot be read is treated as no record. +// Verifies a record that cannot be read is treated as no record: one that is not JSON, and one whose +// version and keys match but one field holds a value of the wrong type, which decoding would +// otherwise fill in only partly. func TestReusableFastCheckRejectsAnUnreadableRecord(t *testing.T) { - directoryPath := t.TempDir() - writeOutputFile(t, directoryPath, "Alpha"+fastResultExtension, "{not json") + for name, write := range map[string]func(t *testing.T, directoryPath string){ + "not json": func(t *testing.T, directoryPath string) { + writeOutputFile(t, directoryPath, "Alpha"+fastResultExtension, "{not json") + }, + "wrong field type": func(t *testing.T, directoryPath string) { + recordCheck(t, directoryPath) + path := fastResultPath(directoryPath, "Alpha") + content, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read the recorded check: %v", err) + } + rewritten := strings.Replace(string(content), `"Line":3`, `"Line":"three"`, 1) + if err := os.WriteFile(path, []byte(rewritten), 0o600); err != nil { + t.Fatalf("failed to write the recorded check back: %v", err) + } + }, + } { + t.Run(name, func(t *testing.T) { + directoryPath := t.TempDir() + write(t, directoryPath) - if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "worker"); found { - t.Error("an unreadable record should not be found") + if _, found := directoryStore(directoryPath).ReusableFastCheck("Alpha", "input", checkedBaseline(), "worker"); found { + t.Error("an unreadable record should not be found") + } + }) } }