From f7e36ae97df19d24ad3d344cf9755d79e2a4cc0a Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:33:58 -0400 Subject: [PATCH 01/10] feat!: track applied migrations as a list instead of a scalar schema_version Every new migration rewrote the single schema_version line to its own timestamp, so any two branches adding migrations conflicted on it. Replace the scalar with a schema_versions list of every applied version, ordered by a hash of each version so concurrently-added migrations scatter through the file instead of clustering on one line. Bumps serializer_version to 2. schema load now uses set membership against the list, and Read rejects schema files written by a newer CLI. This is a breaking format change and needs a coordinated CLI rollout per consumer. Co-Authored-By: Claude Opus 4.8 (1M context) --- migrationmanagers/migrationmanagers.go | 9 +-- schema/schema.go | 22 +++++- schema/schema_test.go | 105 +++++++++++++++++++++++++ schemaloaders/schemaloaders.go | 15 +++- serializers/serializers.go | 23 +++++- 5 files changed, 159 insertions(+), 15 deletions(-) create mode 100644 schema/schema_test.go diff --git a/migrationmanagers/migrationmanagers.go b/migrationmanagers/migrationmanagers.go index 2d43455..d7b83ba 100644 --- a/migrationmanagers/migrationmanagers.go +++ b/migrationmanagers/migrationmanagers.go @@ -94,8 +94,8 @@ func (m *MigrationManager) ApplyToSchema(migrationRepo migrations.Repository, id } appliedVersion := m.migration.MigrationVersion() - if appliedVersion != nil && m.schema.SchemaVersion < *appliedVersion { - m.schema.SchemaVersion = *appliedVersion + if appliedVersion != nil { + m.schema.AddVersion(*appliedVersion) } return nil } @@ -160,10 +160,7 @@ func (m *MigrationManager) SyncVersion() error { return fmt.Errorf("got %d status code", resp.StatusCode) } - appliedVersion := m.migration.MigrationVersion() - if m.schema.SchemaVersion < *appliedVersion { - m.schema.SchemaVersion = *appliedVersion - } + m.schema.AddVersion(*m.migration.MigrationVersion()) return nil } diff --git a/schema/schema.go b/schema/schema.go index 9916da8..43f1c53 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -1,6 +1,7 @@ package schema import ( + "crypto/sha1" "encoding/json" "errors" "fmt" @@ -42,6 +43,12 @@ func Read() (*serializers.Schema, error) { if err != nil { return nil, err } + if schema.SerializerVersion > serializers.SerializerVersion { + return nil, fmt.Errorf( + "%s was written by a newer testtrack CLI (serializer_version %d, this CLI supports %d). Please upgrade your testtrack CLI", + schemaPath, schema.SerializerVersion, serializers.SerializerVersion, + ) + } return &schema, nil } @@ -204,14 +211,23 @@ func applyAllMigrationsToSchema(schema *serializers.Schema) error { return err } } - if len(versions) != 0 { - schema.SchemaVersion = versions[len(versions)-1] - } + schema.SchemaVersions = versions return nil } +// hashVersion returns a stable hash of a migration version, used to order the +// schema_versions list. Ordering by hash scatters newly added versions +// throughout the list instead of clustering them by timestamp, so concurrently +// merged migrations rarely touch the same lines. +func hashVersion(version string) string { + return fmt.Sprintf("%x", sha1.Sum([]byte(version))) +} + // SortAlphabetically sorts the schema's resource slices by their natural keys func SortAlphabetically(schema *serializers.Schema) { + sort.Slice(schema.SchemaVersions, func(i, j int) bool { + return hashVersion(schema.SchemaVersions[i]) < hashVersion(schema.SchemaVersions[j]) + }) sort.Slice(schema.RemoteKills, func(i, j int) bool { return schema.RemoteKills[i].Split < schema.RemoteKills[j].Split && schema.RemoteKills[i].Reason < schema.RemoteKills[j].Reason diff --git a/schema/schema_test.go b/schema/schema_test.go new file mode 100644 index 0000000..64aad12 --- /dev/null +++ b/schema/schema_test.go @@ -0,0 +1,105 @@ +package schema + +import ( + "os" + "path/filepath" + "sort" + "strconv" + "testing" + + "github.com/Betterment/testtrack-cli/serializers" + "github.com/stretchr/testify/require" +) + +func TestSortAlphabeticallyOrdersVersionsByHash(t *testing.T) { + // Timestamp-ascending order; the hash ordering should differ from this so + // that newly appended (newest-timestamp) versions scatter rather than + // always landing at the end of the list. + versions := []string{ + "2020011712345", + "2020011712346", + "2020011712347", + "2020011712348", + "2020011712349", + } + + schema := &serializers.Schema{SchemaVersions: append([]string(nil), versions...)} + SortAlphabetically(schema) + + expected := append([]string(nil), versions...) + sort.Slice(expected, func(i, j int) bool { + return hashVersion(expected[i]) < hashVersion(expected[j]) + }) + require.Equal(t, expected, schema.SchemaVersions, "versions should be ordered by hash") + + require.NotEqual(t, versions, schema.SchemaVersions, + "hash ordering should scatter versions rather than preserve timestamp order") + + // Ordering must be stable across calls so the file doesn't churn. + again := &serializers.Schema{SchemaVersions: append([]string(nil), versions...)} + SortAlphabetically(again) + require.Equal(t, schema.SchemaVersions, again.SchemaVersions, "hash ordering should be deterministic") +} + +func TestReadRejectsNewerSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 99 +schema_versions: +- "2020011712345" +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "upgrade your testtrack CLI") +} + +func TestReadAcceptsCurrentSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 2 +schema_versions: +- "2020011712345" +- "2020011712346" +`) + + schema, err := Read() + require.NoError(t, err) + require.Equal(t, []string{"2020011712345", "2020011712346"}, schema.SchemaVersions) +} + +func TestGenerateRecordsAllMigrationVersions(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + + versions := []string{"2020011712345", "2020011712346", "2020011712347"} + for i, version := range versions { + writeSplitMigration(t, version, []string{"alpha_experiment", "bravo_experiment", "charlie_experiment"}[i]) + } + + schema, err := Generate() + require.NoError(t, err) + + require.ElementsMatch(t, versions, schema.SchemaVersions, + "every migration version should be recorded in schema_versions") +} + +// withSchemaFile writes a schema file into a temp testtrack dir and chdirs there. +func withSchemaFile(t *testing.T, contents string) { + t.Helper() + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack", 0755)) + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(contents), 0644)) +} + +func writeSplitMigration(t *testing.T, version, name string) { + t.Helper() + contents := "serializer_version: " + strconv.Itoa(serializers.SerializerVersion) + "\n" + + "split:\n" + + " name: " + name + "\n" + + " weights:\n" + + " control: 50\n" + + " treatment: 50\n" + path := filepath.Join("testtrack", "migrate", version+"_"+name+".yml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0644)) +} diff --git a/schemaloaders/schemaloaders.go b/schemaloaders/schemaloaders.go index cbf5d22..53a119b 100644 --- a/schemaloaders/schemaloaders.go +++ b/schemaloaders/schemaloaders.go @@ -71,10 +71,19 @@ func (s *SchemaLoader) Load() error { } } + appliedVersions := make(map[string]bool, len(s.schema.SchemaVersions)) + for _, version := range s.schema.SchemaVersions { + appliedVersions[version] = true + } + + warnedOfNewerMigrations := false for _, version := range s.migrationRepo.SortedVersions() { - if version > s.schema.SchemaVersion { - fmt.Println("Schema load complete, but there are migrations newer than the schema file - run testtrack migrate to apply them.") - break + if !appliedVersions[version] { + if !warnedOfNewerMigrations { + fmt.Println("Schema load complete, but there are migrations newer than the schema file - run testtrack migrate to apply them.") + warnedOfNewerMigrations = true + } + continue } err := migrationmanagers.NewWithServer((*s.migrationRepo)[version], s.server).SyncVersion() if err != nil { diff --git a/serializers/serializers.go b/serializers/serializers.go index 34f06ae..b72e120 100644 --- a/serializers/serializers.go +++ b/serializers/serializers.go @@ -4,8 +4,14 @@ import ( "gopkg.in/yaml.v2" ) -// SerializerVersion is the current version of the migration file format so we can evolve over time -const SerializerVersion = 1 +// SerializerVersion is the current version of the migration file format so we can evolve over time. +// +// Version 2 replaced the single scalar schema_version high-water-mark with a +// schema_versions list of every applied migration version, eliminating the +// merge-conflict hotspot that the scalar created (every new migration rewrote +// the same line). The list is ordered by a hash of each version so concurrently +// added migrations scatter through the file instead of clustering. +const SerializerVersion = 2 // MigrationVersion is a JSON-marshalable representation of migration version (timestamp) type MigrationVersion struct { @@ -90,13 +96,24 @@ type SchemaSplit struct { // migration validation and bootstrapping of new ecosystems type Schema struct { SerializerVersion int `yaml:"serializer_version" json:"serializer_version"` - SchemaVersion string `yaml:"schema_version" json:"schema_version"` + SchemaVersions []string `yaml:"schema_versions,omitempty" json:"schema_versions,omitempty"` Splits []SchemaSplit `yaml:"splits,omitempty" json:"splits,omitempty"` IdentifierTypes []IdentifierType `yaml:"identifier_types,omitempty" json:"identifier_types,omitempty"` RemoteKills []RemoteKill `yaml:"remote_kills,omitempty" json:"remote_kills,omitempty"` FeatureCompletions []FeatureCompletion `yaml:"feature_completions,omitempty" json:"feature_completions,omitempty"` } +// AddVersion records a migration version as applied in the schema, ignoring +// duplicates. Ordering is handled at write time, so callers needn't sort. +func (s *Schema) AddVersion(version string) { + for _, v := range s.SchemaVersions { + if v == version { + return + } + } + s.SchemaVersions = append(s.SchemaVersions, version) +} + // LegacySchema represents the Rails migration-piggybacked testtrack schema files of old type LegacySchema struct { IdentifierTypes []string `yaml:"identifier_types"` From c4d041267efbbecc823e6b155498c30f9468c1fb Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:39:54 -0400 Subject: [PATCH 02/10] chore: bump CLI version to 2.0.0 for the schema format change Co-Authored-By: Claude Opus 4.8 (1M context) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8ce3795..3daa91b 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ SHELL = /bin/sh -VERSION=1.8.0 +VERSION=2.0.0 BUILD=`git rev-parse HEAD` LDFLAGS=-ldflags "-w -s \ From 338f65a71a26da8bf80d8203e4af89bd4eaed0d5 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:42:40 -0400 Subject: [PATCH 03/10] test: assert hash ordering against a hard-coded expected slice The previous test computed its expected order with hashVersion itself, so it was circular -- a no-op or buggy hashVersion could still pass. Pin the sha1 ordering as a literal so the test fails if the ordering changes or regresses to a lexical/timestamp sort. Co-Authored-By: Claude Opus 4.8 (1M context) --- schema/schema_test.go | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/schema/schema_test.go b/schema/schema_test.go index 64aad12..53f44c1 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -3,7 +3,6 @@ package schema import ( "os" "path/filepath" - "sort" "strconv" "testing" @@ -12,33 +11,27 @@ import ( ) func TestSortAlphabeticallyOrdersVersionsByHash(t *testing.T) { - // Timestamp-ascending order; the hash ordering should differ from this so - // that newly appended (newest-timestamp) versions scatter rather than - // always landing at the end of the list. - versions := []string{ + // Fed in timestamp-ascending order. The expected order below is the hard-coded + // sha1-of-version ordering (see the sha1 hex in the trailing comments) -- it's + // deliberately not derived from hashVersion so the test fails if the ordering + // function changes, no-ops, or regresses to a plain lexical/timestamp sort. + schema := &serializers.Schema{SchemaVersions: []string{ "2020011712345", "2020011712346", "2020011712347", "2020011712348", "2020011712349", - } - - schema := &serializers.Schema{SchemaVersions: append([]string(nil), versions...)} + }} SortAlphabetically(schema) - expected := append([]string(nil), versions...) - sort.Slice(expected, func(i, j int) bool { - return hashVersion(expected[i]) < hashVersion(expected[j]) - }) - require.Equal(t, expected, schema.SchemaVersions, "versions should be ordered by hash") - - require.NotEqual(t, versions, schema.SchemaVersions, - "hash ordering should scatter versions rather than preserve timestamp order") - - // Ordering must be stable across calls so the file doesn't churn. - again := &serializers.Schema{SchemaVersions: append([]string(nil), versions...)} - SortAlphabetically(again) - require.Equal(t, schema.SchemaVersions, again.SchemaVersions, "hash ordering should be deterministic") + expected := []string{ + "2020011712349", // 0dfad45993f8e2122515814d8e62f676fcb33841 + "2020011712347", // 13af4b988d91f57f71abeb0d657c24a18fba27d7 + "2020011712346", // 529bb729c31584a72d98669cf1daa0b9509379ce + "2020011712348", // a467c4bdcdbdac7030e77951932029e03a8d0920 + "2020011712345", // bef98a4d8faa7aea1b347fb834a25c01be3c44d2 + } + require.Equal(t, expected, schema.SchemaVersions, "versions should be ordered by sha1 of the version") } func TestReadRejectsNewerSerializerVersion(t *testing.T) { From a35d067d03f2b40829305859e03690ac04a16d8e Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:44:39 -0400 Subject: [PATCH 04/10] Reword schema load warning to reflect set-membership check The old 'migrations newer than the schema file' wording came from the scalar high-water-mark model, where any unrecorded migration was strictly newer. With the schema_versions set-membership check it's just 'on disk but not recorded in the schema file' -- not necessarily newer, and not the same as unapplied (this loop never checks server-applied state). Co-Authored-By: Claude Opus 4.8 (1M context) --- schemaloaders/schemaloaders.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/schemaloaders/schemaloaders.go b/schemaloaders/schemaloaders.go index 53a119b..775c49a 100644 --- a/schemaloaders/schemaloaders.go +++ b/schemaloaders/schemaloaders.go @@ -76,12 +76,12 @@ func (s *SchemaLoader) Load() error { appliedVersions[version] = true } - warnedOfNewerMigrations := false + warnedOfUnrecordedMigrations := false for _, version := range s.migrationRepo.SortedVersions() { if !appliedVersions[version] { - if !warnedOfNewerMigrations { - fmt.Println("Schema load complete, but there are migrations newer than the schema file - run testtrack migrate to apply them.") - warnedOfNewerMigrations = true + if !warnedOfUnrecordedMigrations { + fmt.Println("Schema load complete, but there are migrations on disk not recorded in the schema file - run testtrack migrate to apply them.") + warnedOfUnrecordedMigrations = true } continue } From fc6a71de4dd42f82712bfa4f34b9c726ea734e7d Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:53:08 -0400 Subject: [PATCH 05/10] Drop editorializing comment from hash-order test Co-Authored-By: Claude Opus 4.8 (1M context) --- schema/schema_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/schema/schema_test.go b/schema/schema_test.go index 53f44c1..b49c482 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -11,10 +11,6 @@ import ( ) func TestSortAlphabeticallyOrdersVersionsByHash(t *testing.T) { - // Fed in timestamp-ascending order. The expected order below is the hard-coded - // sha1-of-version ordering (see the sha1 hex in the trailing comments) -- it's - // deliberately not derived from hashVersion so the test fails if the ordering - // function changes, no-ops, or regresses to a plain lexical/timestamp sort. schema := &serializers.Schema{SchemaVersions: []string{ "2020011712345", "2020011712346", From 4b6d153ded61e5f6bb58a259bd567df297173413 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 09:57:53 -0400 Subject: [PATCH 06/10] test: cover schema load's unrecorded-migration warning Asserts that load marks only schema-recorded versions applied on the server, skips migration files on disk that aren't recorded, and prints the warning exactly once regardless of how many are unrecorded. Co-Authored-By: Claude Opus 4.8 (1M context) --- schemaloaders/schemaloaders_test.go | 96 +++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 schemaloaders/schemaloaders_test.go diff --git a/schemaloaders/schemaloaders_test.go b/schemaloaders/schemaloaders_test.go new file mode 100644 index 0000000..272993f --- /dev/null +++ b/schemaloaders/schemaloaders_test.go @@ -0,0 +1,96 @@ +package schemaloaders + +import ( + "io" + "net/http" + "os" + "strings" + "testing" + + "github.com/Betterment/testtrack-cli/migrations" + "github.com/Betterment/testtrack-cli/serializers" + "github.com/Betterment/testtrack-cli/splits" + "github.com/stretchr/testify/require" +) + +const unrecordedWarning = "migrations on disk not recorded in the schema file" + +// fakeServer records the paths it's asked to POST and always succeeds. +type fakeServer struct { + postedPaths []string +} + +func (f *fakeServer) Get(string, interface{}) error { return nil } + +func (f *fakeServer) Delete(string) error { return nil } + +func (f *fakeServer) Post(path string, _ interface{}) (*http.Response, error) { + f.postedPaths = append(f.postedPaths, path) + return &http.Response{StatusCode: 204, Body: io.NopCloser(strings.NewReader(""))}, nil +} + +func TestLoadSyncsOnlyRecordedVersionsAndWarnsOnce(t *testing.T) { + server := &fakeServer{} + // Two versions are recorded in the schema; two migration files on disk are not. + schema := &serializers.Schema{SchemaVersions: []string{"2020011712345", "2020011712346"}} + repo := migrations.Repository{ + "2020011712345": newSplitMigration(t, "2020011712345"), + "2020011712346": newSplitMigration(t, "2020011712346"), + "2020011712347": newSplitMigration(t, "2020011712347"), + "2020011712348": newSplitMigration(t, "2020011712348"), + } + loader := &SchemaLoader{server: server, schema: schema, migrationRepo: &repo} + + out := captureStdout(t, func() { + require.NoError(t, loader.Load()) + }) + + require.Equal(t, 1, strings.Count(out, unrecordedWarning), + "warning should print exactly once no matter how many unrecorded migrations exist") + // Only the two recorded versions get marked applied on the server; the + // schema here has no splits/etc., so these are the only posts. + require.Equal(t, []string{"api/v2/migrations", "api/v2/migrations"}, server.postedPaths) +} + +func TestLoadDoesNotWarnWhenEveryMigrationIsRecorded(t *testing.T) { + server := &fakeServer{} + schema := &serializers.Schema{SchemaVersions: []string{"2020011712345", "2020011712346"}} + repo := migrations.Repository{ + "2020011712345": newSplitMigration(t, "2020011712345"), + "2020011712346": newSplitMigration(t, "2020011712346"), + } + loader := &SchemaLoader{server: server, schema: schema, migrationRepo: &repo} + + out := captureStdout(t, func() { + require.NoError(t, loader.Load()) + }) + + require.NotContains(t, out, unrecordedWarning) + require.Equal(t, []string{"api/v2/migrations", "api/v2/migrations"}, server.postedPaths) +} + +func newSplitMigration(t *testing.T, version string) migrations.IMigration { + t.Helper() + migration, err := splits.FromFile(&version, &serializers.SplitYAML{ + Name: "split_" + version, + Weights: map[string]int{"control": 50, "treatment": 50}, + }) + require.NoError(t, err) + return migration +} + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + original := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + defer func() { os.Stdout = original }() + + fn() + require.NoError(t, w.Close()) + + out, err := io.ReadAll(r) + require.NoError(t, err) + return string(out) +} From 840de124ce12e1a8d6660edbc21eec9fdb85b1d1 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 11:27:19 -0400 Subject: [PATCH 07/10] Address code review: harden upgrade path, fix warning, tidy sort - Read() now rejects an older-format schema (serializer_version below current) and points at `testtrack schema generate`, instead of silently round-tripping a v1 file into a lossy v2 write that drops migration history. generate doesn't read the file, so it remains the upgrade path. - Schema load warning now points at `testtrack schema generate` (which updates the file) rather than `testtrack migrate` (which never persists schema_versions, so the warning would recur). - Split version hash-ordering out of SortAlphabetically into sortSchemaVersions so the function name stays honest, and precompute each version's hash once (comparing raw sha1 bytes) instead of recomputing on every comparison. - Update fakeserver fixtures to the v2 schema_versions format. Co-Authored-By: Claude Opus 4.8 (1M context) --- fakeserver/server_test.go | 9 ++++---- schema/schema.go | 40 +++++++++++++++++++++++++--------- schema/schema_test.go | 15 +++++++++++-- schemaloaders/schemaloaders.go | 2 +- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/fakeserver/server_test.go b/fakeserver/server_test.go index a0a554a..de5147a 100644 --- a/fakeserver/server_test.go +++ b/fakeserver/server_test.go @@ -20,8 +20,9 @@ import ( ) var testSchema = ` -serializer_version: 1 -schema_version: "2020011774023" +serializer_version: 2 +schema_versions: +- "2020011774023" splits: - name: test.test_experiment weights: @@ -34,8 +35,8 @@ splits: ` var otherTestSchema = `{ - "serializer_version": 1, - "schema_version": "2020011774023", + "serializer_version": 2, + "schema_versions": ["2020011774023"], "splits": [ { "name": "test.json_experiment", diff --git a/schema/schema.go b/schema/schema.go index 43f1c53..f2f47e9 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -1,6 +1,7 @@ package schema import ( + "bytes" "crypto/sha1" "encoding/json" "errors" @@ -49,6 +50,17 @@ func Read() (*serializers.Schema, error) { schemaPath, schema.SerializerVersion, serializers.SerializerVersion, ) } + if schema.SerializerVersion < serializers.SerializerVersion { + // An older file predates a format change and may be missing data that + // can't be reconstructed from it (e.g. the v1 scalar schema_version + // carried no per-migration list). Refuse to read it rather than + // silently round-trip a lossy upgrade; `schema generate` rebuilds a + // current-format file from the migrations on disk. + return nil, fmt.Errorf( + "%s uses an older schema format (serializer_version %d, this CLI writes %d). Run `testtrack schema generate` to upgrade it", + schemaPath, schema.SerializerVersion, serializers.SerializerVersion, + ) + } return &schema, nil } @@ -70,9 +82,10 @@ func Generate() (*serializers.Schema, error) { return schema, nil } -// Write a schema to disk after alpha-sorting its resources +// Write a schema to disk after sorting its resources into a stable order func Write(schema *serializers.Schema) error { SortAlphabetically(schema) + sortSchemaVersions(schema) schemaPath, _ := findSchemaPath() @@ -215,19 +228,26 @@ func applyAllMigrationsToSchema(schema *serializers.Schema) error { return nil } -// hashVersion returns a stable hash of a migration version, used to order the -// schema_versions list. Ordering by hash scatters newly added versions -// throughout the list instead of clustering them by timestamp, so concurrently -// merged migrations rarely touch the same lines. -func hashVersion(version string) string { - return fmt.Sprintf("%x", sha1.Sum([]byte(version))) +// sortSchemaVersions orders the applied-version list by the SHA-1 of each +// version. The order is otherwise meaningless; hashing scatters newly added +// versions through the list instead of clustering them by timestamp, so two +// branches that each append a migration rarely touch the same lines. Hashes +// are precomputed so each version is hashed once rather than on every +// comparison. +func sortSchemaVersions(schema *serializers.Schema) { + hashes := make(map[string][sha1.Size]byte, len(schema.SchemaVersions)) + for _, version := range schema.SchemaVersions { + hashes[version] = sha1.Sum([]byte(version)) + } + sort.Slice(schema.SchemaVersions, func(i, j int) bool { + a := hashes[schema.SchemaVersions[i]] + b := hashes[schema.SchemaVersions[j]] + return bytes.Compare(a[:], b[:]) < 0 + }) } // SortAlphabetically sorts the schema's resource slices by their natural keys func SortAlphabetically(schema *serializers.Schema) { - sort.Slice(schema.SchemaVersions, func(i, j int) bool { - return hashVersion(schema.SchemaVersions[i]) < hashVersion(schema.SchemaVersions[j]) - }) sort.Slice(schema.RemoteKills, func(i, j int) bool { return schema.RemoteKills[i].Split < schema.RemoteKills[j].Split && schema.RemoteKills[i].Reason < schema.RemoteKills[j].Reason diff --git a/schema/schema_test.go b/schema/schema_test.go index b49c482..3772dde 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestSortAlphabeticallyOrdersVersionsByHash(t *testing.T) { +func TestSortSchemaVersionsByHash(t *testing.T) { schema := &serializers.Schema{SchemaVersions: []string{ "2020011712345", "2020011712346", @@ -18,7 +18,7 @@ func TestSortAlphabeticallyOrdersVersionsByHash(t *testing.T) { "2020011712348", "2020011712349", }} - SortAlphabetically(schema) + sortSchemaVersions(schema) expected := []string{ "2020011712349", // 0dfad45993f8e2122515814d8e62f676fcb33841 @@ -42,6 +42,17 @@ schema_versions: require.Contains(t, err.Error(), "upgrade your testtrack CLI") } +func TestReadRejectsOlderSerializerVersion(t *testing.T) { + withSchemaFile(t, ` +serializer_version: 1 +schema_version: "2020011712345" +`) + + _, err := Read() + require.Error(t, err) + require.Contains(t, err.Error(), "testtrack schema generate") +} + func TestReadAcceptsCurrentSerializerVersion(t *testing.T) { withSchemaFile(t, ` serializer_version: 2 diff --git a/schemaloaders/schemaloaders.go b/schemaloaders/schemaloaders.go index 775c49a..9f1d891 100644 --- a/schemaloaders/schemaloaders.go +++ b/schemaloaders/schemaloaders.go @@ -80,7 +80,7 @@ func (s *SchemaLoader) Load() error { for _, version := range s.migrationRepo.SortedVersions() { if !appliedVersions[version] { if !warnedOfUnrecordedMigrations { - fmt.Println("Schema load complete, but there are migrations on disk not recorded in the schema file - run testtrack migrate to apply them.") + fmt.Println("Schema load complete, but there are migrations on disk not recorded in the schema file - run testtrack schema generate to update it.") warnedOfUnrecordedMigrations = true } continue From e0c171fc7f28e33a732c55e0ea1eee636c6c5d93 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 11:33:51 -0400 Subject: [PATCH 08/10] Add UPGRADING.md for the 1.x -> 2.0.0 schema format change Co-Authored-By: Claude Opus 4.8 (1M context) --- UPGRADING.md | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 UPGRADING.md diff --git a/UPGRADING.md b/UPGRADING.md new file mode 100644 index 0000000..0e7491a --- /dev/null +++ b/UPGRADING.md @@ -0,0 +1,73 @@ +# Upgrading testtrack-cli + +## 1.x to 2.0.0 + +2.0.0 changes the on-disk schema format (`testtrack/schema.{yml,json}`). + +### What changed + +The schema's applied-migration high-water mark used to be a single scalar +field: + +```yaml +serializer_version: 1 +schema_version: "20200115000000" +``` + +Every new migration rewrote that one line, so any two branches that added +migrations conflicted on it. 2.0.0 replaces the scalar with a list of every +applied migration version (`serializer_version` bumps to `2`): + +```yaml +serializer_version: 2 +schema_versions: +- "20200601000000" +- "20200115000000" +``` + +The list is ordered by a hash of each version (not chronologically), so +concurrently added migrations scatter through the file instead of clustering +on one line. The ordering is otherwise meaningless — don't rely on it. + +### This is a breaking format change + +A 2.0.0 CLI **refuses to read** an old (`serializer_version: 1`) schema file — +`create`, `sync`, `migrate`, and `schema load` will error and tell you to +regenerate. This is deliberate: the old scalar can't be losslessly converted in +place, so the CLI makes you rebuild from the migrations on disk rather than +silently dropping your migration history. + +Conversely, a pre-2.0.0 CLI does **not** understand `schema_versions`. If it +reads a v2 schema it ignores the field, and the next time it regenerates the +schema it rewrites the file back to the v1 format. So the whole team — and every +CI/build environment — has to be on 2.0.0 before you commit a v2 schema, or the +format will flip-flop between commits. + +### How to upgrade + +1. **Get everyone onto 2.0.0 first.** Bump the CLI everywhere it runs: each + developer's install (e.g. `brew upgrade testtrack-cli`) **and** every + CI/build image that downloads the `testtrack` binary. Pin those CI downloads + to `v2.0.0`. The build pipeline is the easy one to forget. + +2. **Regenerate and commit the schema, once.** In your app root: + + ```sh + testtrack schema generate + ``` + + This rebuilds `testtrack/schema.{yml,json}` from the migration files in + `testtrack/migrate`, producing the v2 format with every version recorded. + Commit the result. Expect a large one-time diff — the format conversion plus + the hash ordering touches the whole file. + +3. **Done.** Subsequent `create`/`migrate`/`load` work as before, and new + migrations no longer collide on the version line. + +### Notes + +- `schema generate` is the only command that can read a v1 file (it doesn't read + the old schema — it rebuilds from migrations), so it's always the recovery + path if you hit the "older schema format" error. +- No server-side change is required. The TestTrack server tracks applied + migrations itself; `schema_version`/`schema_versions` is a CLI-only artifact. From 2f05e6aaeef4d617c5a509ec14e392b8ac87ca92 Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 11:43:44 -0400 Subject: [PATCH 09/10] Fix UPGRADING.md: atomic CI+schema rollout, complete command list Review caught two inaccuracies: the upgrade sequence (bump CI first, then commit schema) would break testtrack migrate in CI in the interim window, so the CI pin and the v2 schema commit must land together; and the list of commands that error on a v1 schema was incomplete (decide and destroy also go through Read). Co-Authored-By: Claude Opus 4.8 (1M context) --- UPGRADING.md | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 0e7491a..1c176d4 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -31,9 +31,11 @@ on one line. The ordering is otherwise meaningless — don't rely on it. ### This is a breaking format change -A 2.0.0 CLI **refuses to read** an old (`serializer_version: 1`) schema file — -`create`, `sync`, `migrate`, and `schema load` will error and tell you to -regenerate. This is deliberate: the old scalar can't be losslessly converted in +A 2.0.0 CLI **refuses to read** an old (`serializer_version: 1`) schema file. +Any command that reads the schema errors and tells you to regenerate — +`create`, `migrate`, `sync`, `decide`, the `destroy` commands, and `schema +load` (essentially everything except `assign`/`unassign`, which use a different +read path). This is deliberate: the old scalar can't be losslessly converted in place, so the CLI makes you rebuild from the migrations on disk rather than silently dropping your migration history. @@ -43,26 +45,38 @@ schema it rewrites the file back to the v1 format. So the whole team — and eve CI/build environment — has to be on 2.0.0 before you commit a v2 schema, or the format will flip-flop between commits. -### How to upgrade +Because a 2.0.0 CLI rejects a v1 schema and a 1.x CLI reverts a v2 schema, the +switch is a flag day: the moment the v2 schema lands in the repo, every consumer +of it must already be on 2.0.0. Order the rollout so the CLI bump and the schema +commit line up, not in separate steps. -1. **Get everyone onto 2.0.0 first.** Bump the CLI everywhere it runs: each - developer's install (e.g. `brew upgrade testtrack-cli`) **and** every - CI/build image that downloads the `testtrack` binary. Pin those CI downloads - to `v2.0.0`. The build pipeline is the easy one to forget. +1. **Make 2.0.0 available, but don't switch anything to it yet.** Cut the + `v2.0.0` release, update the brew formula, and make the pinned binary + available to CI — without yet changing the CI pin or asking devs to upgrade. -2. **Regenerate and commit the schema, once.** In your app root: +2. **Land the schema regen and the CI bump together, in one change.** In your + app root: ```sh testtrack schema generate ``` This rebuilds `testtrack/schema.{yml,json}` from the migration files in - `testtrack/migrate`, producing the v2 format with every version recorded. - Commit the result. Expect a large one-time diff — the format conversion plus - the hash ordering touches the whole file. - -3. **Done.** Subsequent `create`/`migrate`/`load` work as before, and new - migrations no longer collide on the version line. + `testtrack/migrate` into the v2 format. In the **same** PR, bump the + CI/build pin to `v2.0.0`. They must be atomic: if CI runs 2.0.0 against a + still-v1 schema, `testtrack migrate` errors; if the v2 schema lands while CI + still runs 1.x, the old binary rewrites it back to v1. Expect a large + one-time diff — the format conversion plus the hash ordering touches the + whole file. + +3. **Developers upgrade as that change lands** (`brew upgrade testtrack-cli`). A + developer must be on 2.0.0 before running any testtrack command against the + regenerated schema — an older binary silently reverts it to v1. + +> Note: `schema generate` rebuilds the version list from the migration files +> currently in `testtrack/migrate`. If your repo prunes/squashes old migration +> files, only versions with surviving files are recorded — same as the v1 +> behavior, but worth knowing if you rely on the file as a full history. ### Notes From ebd2915f7c6cbcdb657fc69df8d9d645a90824bb Mon Sep 17 00:00:00 2001 From: Harris Effron Date: Tue, 23 Jun 2026 13:20:19 -0400 Subject: [PATCH 10/10] Add `schema upgrade` for in-place v1 -> v2 conversion `schema generate` rebuilds the schema by replaying every migration, which fails on long-lived apps whose testtrack/migrate predates some splits or references splits created out of band (no create migration to replay). That made the documented v1->v2 upgrade path a dead end for real repos. `schema upgrade` converts in place instead: it keeps the already-materialized body and only rebuilds schema_versions from the migration filenames on disk, then restamps serializer_version. It reads the old file directly, bypassing the read-time version guard. Point the guard's error and UPGRADING.md at it. Co-Authored-By: Claude Opus 4.8 (1M context) --- UPGRADING.md | 48 ++++++++++++++++++++------------------ cmds/schema_upgrade.go | 40 ++++++++++++++++++++++++++++++++ schema/schema.go | 52 ++++++++++++++++++++++++++++++++++++++---- schema/schema_test.go | 41 ++++++++++++++++++++++++++++++++- 4 files changed, 153 insertions(+), 28 deletions(-) create mode 100644 cmds/schema_upgrade.go diff --git a/UPGRADING.md b/UPGRADING.md index 1c176d4..4be7f87 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -32,12 +32,12 @@ on one line. The ordering is otherwise meaningless — don't rely on it. ### This is a breaking format change A 2.0.0 CLI **refuses to read** an old (`serializer_version: 1`) schema file. -Any command that reads the schema errors and tells you to regenerate — -`create`, `migrate`, `sync`, `decide`, the `destroy` commands, and `schema -load` (essentially everything except `assign`/`unassign`, which use a different -read path). This is deliberate: the old scalar can't be losslessly converted in -place, so the CLI makes you rebuild from the migrations on disk rather than -silently dropping your migration history. +Any command that reads the schema errors and tells you to run `testtrack schema +upgrade` — `create`, `migrate`, `sync`, `decide`, the `destroy` commands, and +`schema load` (essentially everything except `assign`/`unassign`, which use a +different read path). This is deliberate: the old format can't be losslessly +read into the new struct, so the CLI makes you convert it explicitly rather than +silently round-tripping a lossy upgrade. Conversely, a pre-2.0.0 CLI does **not** understand `schema_versions`. If it reads a v2 schema it ignores the field, and the next time it regenerates the @@ -54,34 +54,38 @@ commit line up, not in separate steps. `v2.0.0` release, update the brew formula, and make the pinned binary available to CI — without yet changing the CI pin or asking devs to upgrade. -2. **Land the schema regen and the CI bump together, in one change.** In your - app root: +2. **Land the schema conversion and the CI bump together, in one change.** In + your app root: ```sh - testtrack schema generate + testtrack schema upgrade ``` - This rebuilds `testtrack/schema.{yml,json}` from the migration files in - `testtrack/migrate` into the v2 format. In the **same** PR, bump the + This converts `testtrack/schema.{yml,json}` to the v2 format **in place**: it + keeps the materialized state already in the file (splits, decisions, + retirements, etc.) and only rebuilds the `schema_versions` list from the + migration filenames in `testtrack/migrate`. In the **same** PR, bump the CI/build pin to `v2.0.0`. They must be atomic: if CI runs 2.0.0 against a still-v1 schema, `testtrack migrate` errors; if the v2 schema lands while CI still runs 1.x, the old binary rewrites it back to v1. Expect a large - one-time diff — the format conversion plus the hash ordering touches the - whole file. + one-time diff — the new `schema_versions` block lists every migration — but + the splits body is left untouched. 3. **Developers upgrade as that change lands** (`brew upgrade testtrack-cli`). A developer must be on 2.0.0 before running any testtrack command against the - regenerated schema — an older binary silently reverts it to v1. - -> Note: `schema generate` rebuilds the version list from the migration files -> currently in `testtrack/migrate`. If your repo prunes/squashes old migration -> files, only versions with surviving files are recorded — same as the v1 -> behavior, but worth knowing if you rely on the file as a full history. + upgraded schema — an older binary silently reverts it to v1. ### Notes -- `schema generate` is the only command that can read a v1 file (it doesn't read - the old schema — it rebuilds from migrations), so it's always the recovery - path if you hit the "older schema format" error. +- **Use `schema upgrade`, not `schema generate`, to convert.** `generate` + rebuilds the schema by replaying every migration from scratch, which fails on + many long-lived apps — e.g. when `testtrack/migrate` predates some splits, or + contains a decision/retirement for a split that was created out of band in the + TestTrack admin (so there's no create migration to replay). `upgrade` doesn't + replay; it preserves the already-correct materialized state and just restamps + the format, so it works regardless. +- `schema upgrade` reads the old file directly (it's the one command that + bypasses the version guard), so it's always the recovery path if you hit the + "older schema format" error. - No server-side change is required. The TestTrack server tracks applied migrations itself; `schema_version`/`schema_versions` is a CLI-only artifact. diff --git a/cmds/schema_upgrade.go b/cmds/schema_upgrade.go new file mode 100644 index 0000000..de83502 --- /dev/null +++ b/cmds/schema_upgrade.go @@ -0,0 +1,40 @@ +package cmds + +import ( + "github.com/Betterment/testtrack-cli/schema" + "github.com/spf13/cobra" +) + +var schemaUpgradeDoc = ` +Upgrades testtrack/schema.{json,yml} to the schema format this CLI writes, +in place, without replaying migrations. + +It keeps the materialized state already in the file (splits, decisions, +retirements, feature completions, identifier types) and only rebuilds the +applied-migration version list from the files in testtrack/migrate. This is the +upgrade path to use when a newer CLI refuses to read an older-format schema. + +Unlike 'schema generate', upgrade does not rebuild the schema from scratch, so +it works on apps whose migrations can't be replayed cleanly - e.g. when +testtrack/migrate predates some splits, or references splits that were created +out of band in the TestTrack admin. +` + +func init() { + schemaCmd.AddCommand(schemaUpgradeCmd) +} + +var schemaUpgradeCmd = &cobra.Command{ + Use: "upgrade", + Short: "Upgrade schema.{json,yml} to the current format in place", + Long: schemaUpgradeDoc, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return schemaUpgrade() + }, +} + +func schemaUpgrade() error { + _, err := schema.Upgrade() + return err +} diff --git a/schema/schema.go b/schema/schema.go index f2f47e9..bbca00c 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -52,18 +52,60 @@ func Read() (*serializers.Schema, error) { } if schema.SerializerVersion < serializers.SerializerVersion { // An older file predates a format change and may be missing data that - // can't be reconstructed from it (e.g. the v1 scalar schema_version - // carried no per-migration list). Refuse to read it rather than - // silently round-trip a lossy upgrade; `schema generate` rebuilds a - // current-format file from the migrations on disk. + // can't be reconstructed by re-reading it (e.g. the v1 scalar + // schema_version carried no per-migration list). Refuse to read it + // rather than silently round-trip a lossy upgrade; `schema upgrade` + // converts it in place, preserving the materialized state. return nil, fmt.Errorf( - "%s uses an older schema format (serializer_version %d, this CLI writes %d). Run `testtrack schema generate` to upgrade it", + "%s uses an older schema format (serializer_version %d, this CLI writes %d). Run `testtrack schema upgrade` to upgrade it", schemaPath, schema.SerializerVersion, serializers.SerializerVersion, ) } return &schema, nil } +// Upgrade converts an existing schema file to the current serializer format in +// place. Unlike Generate it does not replay migrations, so it preserves the +// already-materialized state (splits, decisions, retirements, etc.) and works +// even on schemas that can't be rebuilt from scratch — e.g. apps whose +// testtrack/migrate predates some splits or references ones created out of +// band. The only thing it rebuilds is the applied-version list, which it reads +// from the migration filenames on disk. +func Upgrade() (*serializers.Schema, error) { + schemaPath, exists := findSchemaPath() + if !exists { + return nil, errors.New("no testtrack schema file to upgrade. Run testtrack schema generate to create one") + } + schemaBytes, err := os.ReadFile(schemaPath) + if err != nil { + return nil, err + } + var schema serializers.Schema + err = yaml.Unmarshal(schemaBytes, &schema) + if err != nil { + return nil, err + } + if schema.SerializerVersion > serializers.SerializerVersion { + return nil, fmt.Errorf( + "%s was written by a newer testtrack CLI (serializer_version %d, this CLI writes %d). Please upgrade your testtrack CLI", + schemaPath, schema.SerializerVersion, serializers.SerializerVersion, + ) + } + + migrationRepo, err := migrationloaders.Load() + if err != nil { + return nil, err + } + schema.SchemaVersions = migrationRepo.SortedVersions() + schema.SerializerVersion = serializers.SerializerVersion + + err = Write(&schema) + if err != nil { + return nil, err + } + return &schema, nil +} + // Generate a schema from migrations on the filesystem and write it to disk func Generate() (*serializers.Schema, error) { schema := &serializers.Schema{SerializerVersion: serializers.SerializerVersion} diff --git a/schema/schema_test.go b/schema/schema_test.go index 3772dde..a9a8810 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -50,7 +50,46 @@ schema_version: "2020011712345" _, err := Read() require.Error(t, err) - require.Contains(t, err.Error(), "testtrack schema generate") + require.Contains(t, err.Error(), "testtrack schema upgrade") +} + +func TestUpgradeConvertsInPlacePreservingBodyWithoutReplaying(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + require.NoError(t, os.MkdirAll("testtrack/migrate", 0755)) + + // A v1 schema whose body references a split that has NO create migration on + // disk — i.e. it could not be rebuilt by replaying (the retail case). + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "schema.yml"), []byte(` +serializer_version: 1 +schema_version: "2020011712346" +splits: +- name: legacy_split_created_out_of_band + weights: + "false": 100 + "true": 0 +`), 0644)) + + // Migration files on disk decide that orphan split — replaying these from + // scratch would fail, but upgrade doesn't replay. + writeSplitMigration(t, "2020011712345", "some_other_split") + require.NoError(t, os.WriteFile(filepath.Join("testtrack", "migrate", "2020011712346_create_split_decision_legacy_split_created_out_of_band.yml"), + []byte("serializer_version: 1\nsplit_decision:\n split: legacy_split_created_out_of_band\n variant: \"false\"\n"), 0644)) + + schema, err := Upgrade() + require.NoError(t, err) + + require.Equal(t, serializers.SerializerVersion, schema.SerializerVersion) + // Version list rebuilt from the migration filenames. + require.ElementsMatch(t, []string{"2020011712345", "2020011712346"}, schema.SchemaVersions) + // Materialized body preserved as-is (not replayed/rebuilt). + require.Len(t, schema.Splits, 1) + require.Equal(t, "legacy_split_created_out_of_band", schema.Splits[0].Name) + + // And the file is now readable by the normal guarded Read(). + reread, err := Read() + require.NoError(t, err) + require.Equal(t, serializers.SerializerVersion, reread.SerializerVersion) } func TestReadAcceptsCurrentSerializerVersion(t *testing.T) {