This is an automated email from the ASF dual-hosted git repository. tisonkun pushed a commit to branch codex/skip-snapshot-revision-noise in repository https://gitbox.apache.org/repos/asf/datasketches-tck.git
commit 9277b469aa066dd3d6ddd687820c2ae73fd3b5ef Author: tison <[email protected]> AuthorDate: Mon Oct 5 10:22:22 2026 +0800 fix: skip revision-only and unstable-only snapshot updates --- .github/workflows/update-snapshots.yml | 8 +++--- cmd/tck/__snapshots__/output_test.snap | 15 ++++++++++- cmd/tck/output.go | 37 +++++++++---------------- cmd/tck/output_test.go | 25 ++++++++++++++++- cmd/tck/snapshots.go | 2 +- docs/serialization-snapshots.md | 19 +++++++------ internal/snapshots/reconcile.go | 12 +++------ internal/snapshots/reconcile_test.go | 49 +++++++++++++++++++--------------- 8 files changed, 97 insertions(+), 70 deletions(-) diff --git a/.github/workflows/update-snapshots.yml b/.github/workflows/update-snapshots.yml index 36c2a83..7e16ba2 100644 --- a/.github/workflows/update-snapshots.yml +++ b/.github/workflows/update-snapshots.yml @@ -13,9 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Publishes the latest self-tested snapshots from every source language, so -# that main always holds the newest snapshot set. This workflow is not a -# required check. +# Publishes self-tested snapshots when files are added, deleted, or stable +# contents change. Other sources retain their snapshots and pins together. +# This workflow is not a required check. name: Update snapshots @@ -73,7 +73,7 @@ jobs: { echo "chore: update snapshots to the latest upstream revisions" echo - grep -E '^(Update |Summary:|Source revision:)' "$RUNNER_TEMP/snapshot-update.log" || true + grep -E '^(Update |Summary:|Source revision:|✓)' "$RUNNER_TEMP/snapshot-update.log" || true } > "$RUNNER_TEMP/snapshot-commit-message.txt" git commit --quiet --file "$RUNNER_TEMP/snapshot-commit-message.txt" git push origin HEAD:${{ github.ref_name }} diff --git a/cmd/tck/__snapshots__/output_test.snap b/cmd/tck/__snapshots__/output_test.snap index df92ef2..31209ac 100644 --- a/cmd/tck/__snapshots__/output_test.snap +++ b/cmd/tck/__snapshots__/output_test.snap @@ -33,6 +33,19 @@ Summary: 1 added · 1 modified (1 unstable) · 1 deleted │ │ │ │ │ new 12 B · bbbbbbbbbbbb │ ╰────────┴───────────┴───────┴──────────────────┴─────────────────────────╯ Summary: 0 added · 1 modified (1 unstable) · 0 deleted -✓ Skipped snapshots: source revision unchanged; 1 unstable content modification was not written. +Source revision: keeping a (generated b) +✓ Skipped snapshots: snapshot set and stable contents are unchanged; kept existing snapshots and source revision. + +--- + +[TestPrintUpdateResultWithIdenticalSnapshots/adopt_revision - 1] +Source revision: a -> b +✓ Updated source revision for snapshots; snapshot contents are unchanged. + +--- + +[TestPrintUpdateResultWithIdenticalSnapshots/retain_revision - 1] +Source revision: keeping a (generated b) +✓ Skipped snapshots: snapshot set and stable contents are unchanged; kept existing snapshots and source revision. --- diff --git a/cmd/tck/output.go b/cmd/tck/output.go index 53cb32d..9c03df4 100644 --- a/cmd/tck/output.go +++ b/cmd/tck/output.go @@ -54,11 +54,20 @@ func printUpdateResult(output io.Writer, root string, result snapshots.Result, s if err := printChanges(output, result); err != nil { return err } + skipped := skipUnstableOnly && !result.HasBlockingChanges() + if result.RevisionChanged() { + format := "Source revision: %s -> %s\n" + if skipped { + format = "Source revision: keeping %s (generated %s)\n" + } + if _, err := fmt.Fprintf(output, format, result.PreviousRevision, result.Revision); err != nil { + return err + } + } target := displayPath(root, result.Target) switch { - case skipUnstableOnly && result.UnstableOnly(): - _, err := fmt.Fprintf(output, "✓ Skipped %s: source revision unchanged; %d unstable content %s not written.\n", - target, len(result.Changes), plural(len(result.Changes), "modification was", "modifications were")) + case skipped: + _, err := fmt.Fprintf(output, "✓ Skipped %s: snapshot set and stable contents are unchanged; kept existing snapshots and source revision.\n", target) return err case result.RevisionChanged() && len(result.Changes) == 0: _, err := fmt.Fprintf(output, "✓ Updated source revision for %s; snapshot contents are unchanged.\n", target) @@ -74,9 +83,6 @@ func printUpdateResult(output io.Writer, root string, result snapshots.Result, s func printChanges(output io.Writer, result snapshots.Result) error { if len(result.Changes) == 0 { - if result.RevisionChanged() { - return printRevisionChange(output, result) - } return nil } counts := make(map[snapshots.ChangeStatus]int) @@ -104,30 +110,13 @@ func printChanges(output io.Writer, result snapshots.Result) error { if _, err := fmt.Fprintln(output, changeTable.Render()); err != nil { return err } - if _, err := fmt.Fprintf( + _, err := fmt.Fprintf( output, "Summary: %d added · %d modified (%d unstable) · %d deleted\n", counts[snapshots.ChangeAdded], counts[snapshots.ChangeModified], unstableModified, counts[snapshots.ChangeDeleted], - ); err != nil { - return err - } - if result.RevisionChanged() { - if err := printRevisionChange(output, result); err != nil { - return err - } - } - return nil -} - -func printRevisionChange(output io.Writer, result snapshots.Result) error { - _, err := fmt.Fprintf( - output, - "Source revision: %s -> %s\n", - result.PreviousRevision, - result.Revision, ) return err } diff --git a/cmd/tck/output_test.go b/cmd/tck/output_test.go index 3c529d4..1fc117d 100644 --- a/cmd/tck/output_test.go +++ b/cmd/tck/output_test.go @@ -82,7 +82,7 @@ func TestPrintUpdateResultReportsSkippedChanges(t *testing.T) { result := snapshots.Result{ Target: "snapshots", PreviousRevision: "a", - Revision: "a", + Revision: "b", Changes: []snapshots.Change{{ Status: snapshots.ChangeModified, Stability: snapshots.Unstable, Path: "probabilistic.sk", Before: &before, After: &after, @@ -94,6 +94,29 @@ func TestPrintUpdateResultReportsSkippedChanges(t *testing.T) { snaps.WithConfig(snaps.Raw()).MatchSnapshot(t, output.String()) } +func TestPrintUpdateResultWithIdenticalSnapshots(t *testing.T) { + t.Parallel() + + for _, test := range []struct { + name string + skip bool + }{ + {name: "adopt revision"}, + {name: "retain revision", skip: true}, + } { + t.Run(test.name, func(t *testing.T) { + result := snapshots.Result{ + Target: "snapshots", + PreviousRevision: "a", + Revision: "b", + } + var output bytes.Buffer + require.NoError(t, printUpdateResult(&output, ".", result, test.skip)) + snaps.WithConfig(snaps.Raw()).MatchSnapshot(t, output.String()) + }) + } +} + func testMetadata(size int64, fill byte) snapshots.FileMetadata { metadata := snapshots.FileMetadata{Size: size} for index := range metadata.SHA256 { diff --git a/cmd/tck/snapshots.go b/cmd/tck/snapshots.go index ab7db11..0b99931 100644 --- a/cmd/tck/snapshots.go +++ b/cmd/tck/snapshots.go @@ -131,7 +131,7 @@ func newSnapshotUpdateCommand() *cobra.Command { }, } command.Flags().BoolVar(&skipUnstableOnly, "skip-unstable-only", false, - "Skip a source when its revision is unchanged and only existing unstable snapshots differ") + "Keep existing snapshots and source revision unless files are added, deleted, or stable contents change") return command } diff --git a/docs/serialization-snapshots.md b/docs/serialization-snapshots.md index 5d076ae..dd6a53e 100644 --- a/docs/serialization-snapshots.md +++ b/docs/serialization-snapshots.md @@ -21,7 +21,7 @@ The serialization corpus is a compatibility boundary between DataSketches implem The DataSketches implementation repositories use the same `serialization_test_data/<language>_generated_files` layout for their cross-language fixtures, so a TCK checkout can serve directly as an implementation's test data root, and moving files between the TCK and an implementation is a plain copy. -The `main` branch of this repository publishes the latest self-tested snapshots from every source language. Each language validates its own snapshots with its generators and round-trip tests before they reach the TCK, so a snapshot set that passes there is ready to publish. Whether each implementation can read the others' snapshots is then answered by running its cross-language tests against the latest set, not by reviewing the `.sk` files by hand. +The `main` branch of this repository publishes self-tested snapshot updates from every source language when the file set or stable contents change. Each language validates its own snapshots with its generators and round-trip tests before they reach the TCK, so a snapshot set that passes there is ready to publish. Whether each implementation can read the others' snapshots is then answered by running its cross-language tests against the latest set, not by reviewing the `.sk` files by hand. `config.toml` records the repository and exact commit each snapshot directory was generated from, so any TCK commit remains reproducible: @@ -50,11 +50,11 @@ Update is the only command that writes snapshots. It accepts an optional languag A revision can only be supplied after a language. To regenerate or repair snapshots at an existing pin, pass the commit recorded in `config.toml` explicitly. Updating to an older revision follows the same process as updating to a newer one. If the upstream build or output layout changed, update the corresponding adapter in `internal/snapshots/<language>.go` before running the command. -Every selected source is generated in a temporary workspace and compared before any repository files are replaced. A generation or staging failure aborts the whole batch. Successful updates publish the selected snapshot directories and their exact commits in `config.toml` together. A changed revision with byte-identical snapshots only changes the pin, after generation has verified those contents. +Every selected source is generated in a temporary workspace and compared before any repository files are replaced. A generation or staging failure aborts the whole batch. Successful updates publish the selected snapshot directories and their exact commits in `config.toml` together. By default, a changed revision with byte-identical snapshots only changes the pin, after generation has verified those contents. ### Skip unstable-only changes -`stable` snapshots have deterministic contents. `unstable` snapshots are known to vary between generations, even at the same source commit. By default, update adopts all generated differences, including unstable contents. Add `--skip-unstable-only` to any update invocation to avoid rewriting a source whose commit is unchanged and whose only differences are modifications to existing unstable files: +`stable` snapshots have deterministic contents. `unstable` snapshots are known to vary between generations, even at the same source commit. By default, update adopts all generated differences, including unstable contents. Add `--skip-unstable-only` to any update invocation to adopt a source only when files are added or deleted, or stable contents change, even if the requested revision differs from the current pin: ```shell mise run tck -- snapshots update --skip-unstable-only @@ -62,15 +62,18 @@ mise run tck -- snapshots update go --skip-unstable-only mise run tck -- snapshots update go v0.2.0 --skip-unstable-only ``` -The option applies independently to each selected language. It skips that source's entire update; it never filters individual files from an adopted snapshot set. A revision change, any added or deleted file, or any stable content change causes the complete generated snapshot set to be adopted, including its unstable files. The report identifies skipped sources and shows observed differences even when they were not written. +The option applies independently to each selected language. If the file set and stable contents are unchanged, the entire snapshot directory and its pin in `config.toml` are retained together. This includes byte-identical output at a different revision. The pin continues to identify the source of the retained corpus, rather than the last revision checked, so skipping an update does not introduce drift. Omit the flag to intentionally adopt a revision even when it has no stable snapshot changes. + +Any added or deleted file, including an unstable file, or any stable content change causes the complete generated snapshot set and its revision to be adopted, including its unstable files. The report shows observed differences and explicitly distinguishes a retained pin from the generated revision for skipped sources. | Source revision | Snapshot differences | Default | With `--skip-unstable-only` | |-----------------|------------------------------------------|------------------------------|------------------------------| | Unchanged | None | No changes | No changes | -| Unchanged | Only existing unstable contents | Replace snapshots | Keep the existing snapshots | +| Unchanged | Only existing unstable contents | Replace snapshots | Keep snapshots and pin | | Unchanged | Additions, deletions, or stable contents | Replace snapshots | Replace snapshots | -| Changed | None | Update the pin | Update the pin | -| Changed | Any | Update the pin and snapshots | Update the pin and snapshots | +| Changed | None | Update the pin | Keep snapshots and pin | +| Changed | Only existing unstable contents | Update the pin and snapshots | Keep snapshots and pin | +| Changed | Additions, deletions, or stable contents | Update the pin and snapshots | Update the pin and snapshots | ### Publication and failures @@ -143,7 +146,7 @@ The stable/unstable classification controls check results and the optional unsta `.github/workflows/check.yml` runs `mise run check` for pull requests and pushes to `main`. It validates the Go implementation of the TCK tooling, but it does not run the upstream snapshot generators or modify committed snapshots. -`.github/workflows/update-snapshots.yml` runs `snapshots update --skip-unstable-only` every Monday and on manual dispatch. It is not a required check. The job summary lists each source's revision change, change counts, and skipped updates. The workflow commits and pushes the batch only after the command succeeds; any failed source prevents publication of the whole batch. +`.github/workflows/update-snapshots.yml` runs `snapshots update --skip-unstable-only` every Monday and on manual dispatch. It is not a required check. Every run generates from upstream `HEAD`, but only sources with file additions, deletions, or stable content changes are adopted. The job summary and commit message include change counts, adopted or retained revisions, and skipped updates. The workflow commits and pushes the batch only after the command succeeds; any failed source prevents [...] ## Implementation notes diff --git a/internal/snapshots/reconcile.go b/internal/snapshots/reconcile.go index 4de015c..b38f7be 100644 --- a/internal/snapshots/reconcile.go +++ b/internal/snapshots/reconcile.go @@ -54,12 +54,6 @@ func (result Result) BlockingChangeCount() int { return count } -// UnstableOnly reports whether the only differences are content changes to -// existing unstable snapshots. A revision change always requires an update. -func (result Result) UnstableOnly() bool { - return !result.RevisionChanged() && len(result.Changes) > 0 && !result.HasBlockingChanges() -} - // Check regenerates a source at its configured commit without writing repository // files. Snapshot differences are returned in Result rather than as errors. func Check(ctx context.Context, root, language string, stdout, stderr io.Writer) (Result, error) { @@ -81,8 +75,8 @@ func Check(ctx context.Context, root, language string, stdout, stderr io.Writer) // Update generates every selected source before publishing any changes. Empty // language selects all sources; empty revision selects each remote's HEAD. -// With skipUnstableOnly, unchanged revisions with only unstable content changes -// retain their entire snapshot directory. Other sources are updated in full. +// With skipUnstableOnly, only additions, deletions, or stable content changes +// cause a source's snapshots and pin to be adopted; otherwise both are retained. func Update(ctx context.Context, root, language, revision string, skipUnstableOnly bool, stdout, stderr io.Writer) ([]Result, error) { languages := Languages() if language != "" { @@ -115,7 +109,7 @@ func Update(ctx context.Context, root, language, revision string, skipUnstableOn return nil, fmt.Errorf("update aborted before publishing: %w", err) } results = append(results, result) - if skipUnstableOnly && result.UnstableOnly() { + if skipUnstableOnly && !result.HasBlockingChanges() { continue } if result.RevisionChanged() { diff --git a/internal/snapshots/reconcile_test.go b/internal/snapshots/reconcile_test.go index 17f4f2b..3969bd7 100644 --- a/internal/snapshots/reconcile_test.go +++ b/internal/snapshots/reconcile_test.go @@ -85,22 +85,12 @@ func TestUpdateGenerationFailureLeavesEntireBatchUntouched(t *testing.T) { } func TestUpdateSkipsUnstableSourceWithinBatch(t *testing.T) { - root, _, newRevision := updateFixture(t) - config, _, err := loadConfig(root) - require.NoError(t, err) - require.NoError(t, config.setCommit("go", newRevision)) - content, err := encodeConfig(config) - require.NoError(t, err) - writeTestFile(t, filepath.Join(root, configFilename), string(content)) + root, oldRevision, newRevision := updateFixture(t) target := snapshotDirectory(root, "go") - writeTestFile(t, filepath.Join(target, "stable.sk"), "new") writeTestFile(t, filepath.Join(target, "unstable.sk"), "old") definition := generators["go"] - generate := definition.generate - definition.generate = func(ctx context.Context, paths generationPaths, runner commandRunner) error { - if err := generate(ctx, paths, runner); err != nil { - return err - } + definition.generate = func(_ context.Context, paths generationPaths, _ commandRunner) error { + writeTestFile(t, filepath.Join(paths.destination, "stable.sk"), "old") writeTestFile(t, filepath.Join(paths.destination, "unstable.sk"), "new") return nil } @@ -111,12 +101,17 @@ func TestUpdateSkipsUnstableSourceWithinBatch(t *testing.T) { return Stable } generators["go"] = definition - _, err = Update(t.Context(), root, "", "", true, io.Discard, io.Discard) + _, err := Update(t.Context(), root, "", "", true, io.Discard, io.Discard) require.NoError(t, err) - config, _, err = loadConfig(root) + config, _, err := loadConfig(root) require.NoError(t, err) for _, language := range Languages() { source, _ := config.source(language) + if language == "go" { + require.Equal(t, oldRevision, source.Commit) + requireTestFile(t, filepath.Join(target, "stable.sk"), "old") + continue + } require.Equal(t, newRevision, source.Commit) requireTestFile(t, filepath.Join(snapshotDirectory(root, language), "stable.sk"), "new") } @@ -132,13 +127,16 @@ func TestUpdateUnstableOnlyPolicy(t *testing.T) { keepOld bool }{ {name: "default refreshes unstable contents", generated: map[string]string{"stable.sk": "old", "unstable.sk": "new"}}, + {name: "default adopts new revision with unstable contents", advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new"}}, + {name: "default adopts revision only", advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "old"}}, {name: "skip retains unchanged source", skip: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new"}, keepOld: true}, - {name: "new revision includes unstable contents", skip: true, advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new"}}, + {name: "skip retains old revision with unstable contents", skip: true, advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new"}, keepOld: true}, {name: "stable change includes unstable contents", skip: true, generated: map[string]string{"stable.sk": "new", "unstable.sk": "new"}}, - {name: "unstable addition is adopted", skip: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new", "added.sk": "new"}}, - {name: "unstable deletion is adopted", skip: true, generated: map[string]string{"stable.sk": "old"}}, - {name: "revision only", skip: true, advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "old"}}, - {name: "no changes", skip: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "old"}}, + {name: "stable change adopts new revision and unstable contents", skip: true, advance: true, generated: map[string]string{"stable.sk": "new", "unstable.sk": "new"}}, + {name: "unstable addition is adopted", skip: true, advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "new", "added.sk": "new"}}, + {name: "unstable deletion is adopted", skip: true, advance: true, generated: map[string]string{"stable.sk": "old"}}, + {name: "skip retains old revision with identical contents", skip: true, advance: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "old"}, keepOld: true}, + {name: "no changes", skip: true, generated: map[string]string{"stable.sk": "old", "unstable.sk": "old"}, keepOld: true}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -163,17 +161,24 @@ func TestUpdateUnstableOnlyPolicy(t *testing.T) { if test.advance { revision = newRevision } + _, before, err := loadConfig(root) + require.NoError(t, err) results, err := Update(t.Context(), root, "go", revision, test.skip, io.Discard, io.Discard) require.NoError(t, err) require.Len(t, results, 1) - config, _, err := loadConfig(root) + require.Equal(t, oldRevision, results[0].PreviousRevision) + require.Equal(t, revision, results[0].Revision) + config, after, err := loadConfig(root) require.NoError(t, err) - require.Equal(t, revision, config.Snapshot.Go.Commit) require.Equal(t, oldRevision, config.Snapshot.CPP.Commit) require.Equal(t, oldRevision, config.Snapshot.Java.Commit) expected := test.generated if test.keepOld { expected = map[string]string{"stable.sk": "old", "unstable.sk": "old"} + require.Equal(t, string(before), string(after)) + require.Equal(t, oldRevision, config.Snapshot.Go.Commit) + } else { + require.Equal(t, revision, config.Snapshot.Go.Commit) } entries, err := os.ReadDir(target) require.NoError(t, err) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
