diff --git a/.golangci.yml b/.golangci.yml index 1bd7b68a..09b52815 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -113,13 +113,14 @@ linters: - linters: - noctx path: _test\.go - # misspell: "cancelled" is the canonical stored status value in - # pkg/ladder (RunStatusCancelled, TrancheStatusCancelled); allow the UK - # spelling in the files that carry it. + # misspell: pkg/ladder writes "canceled" but still reads the legacy + # "cancelled" that rows carried before #554, and keeps the deprecated + # RunStatusCancelled/TrancheStatusCancelled aliases; allow the UK spelling + # in the files that carry it. - linters: - misspell - path: pkg/ladder/(store|types_test)\.go - text: "cancelled" + path: pkg/ladder/(store|types_test|canceled_spelling_test)\.go + text: "(?i)cancelled" paths: - third_party$ - builtin$ diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b6c14b..bdb07c9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). `cache.PurchaseCommitment` now refuses before any API call with an error wrapping `common.ErrRefusedBeforePurchase`. Consumers that buy an Azure `ServiceCache` recommendation must route it to managedredis (#538). +- Ladder run and tranche statuses now write the US spelling `canceled` + (`RunStatusCanceled`, `TrancheStatusCanceled`). Rows holding the legacy + `cancelled` still validate, count as terminal, and parse to the new + constants through `ParseRunStatus` and `ParseTrancheStatus`. The + `RunStatusCancelled` and `TrancheStatusCancelled` names remain as + deprecated aliases with the new value. Consumers that compare raw stored + strings or filter on `cancelled` in SQL must accept both spellings. - `ListTargetOfferings` now walks up to 50 pages (about 5000 offerings) instead of 10, and returns an error wrapping the new `ec2.ErrTargetOfferingsTruncated` (with no list) when that bound is hit, diff --git a/pkg/ladder/canceled_spelling_test.go b/pkg/ladder/canceled_spelling_test.go new file mode 100644 index 00000000..804521d1 --- /dev/null +++ b/pkg/ladder/canceled_spelling_test.go @@ -0,0 +1,42 @@ +package ladder + +import "testing" + +// #554: writers emit the US spelling "canceled"; rows written before keep the +// legacy "cancelled" and still parse, validate and count as terminal. + +func TestCanceledStatuses_WriteTheUSSpelling(t *testing.T) { + t.Parallel() + if RunStatusCanceled != "canceled" || TrancheStatusCanceled != "canceled" { + t.Fatalf("writers must emit \"canceled\": run %q, tranche %q", RunStatusCanceled, TrancheStatusCanceled) + } + if RunStatusCancelled != RunStatusCanceled || TrancheStatusCancelled != TrancheStatusCanceled { + t.Fatal("the deprecated Cancelled aliases must equal the Canceled constants") + } +} + +func TestCanceledStatuses_LegacySpellingParsesToCanceled(t *testing.T) { + t.Parallel() + for _, in := range []string{"canceled", "cancelled"} { + run, err := ParseRunStatus(in) + if err != nil || run != RunStatusCanceled { + t.Errorf("ParseRunStatus(%q) = %q, %v; want %q", in, run, err, RunStatusCanceled) + } + tr, err := ParseTrancheStatus(in) + if err != nil || tr != TrancheStatusCanceled { + t.Errorf("ParseTrancheStatus(%q) = %q, %v; want %q", in, tr, err, TrancheStatusCanceled) + } + if err := RunStatus(in).Validate(); err != nil { + t.Errorf("RunStatus(%q).Validate() = %v; a stored legacy row must still validate", in, err) + } + if err := TrancheStatus(in).Validate(); err != nil { + t.Errorf("TrancheStatus(%q).Validate() = %v", in, err) + } + if !RunStatus(in).IsTerminal() { + t.Errorf("RunStatus(%q).IsTerminal() = false", in) + } + if TrancheStatus(in).allowsFiredAt() { + t.Errorf("TrancheStatus(%q) must not allow FiredAt", in) + } + } +} diff --git a/pkg/ladder/store.go b/pkg/ladder/store.go index e9227c48..48234e34 100644 --- a/pkg/ladder/store.go +++ b/pkg/ladder/store.go @@ -17,8 +17,16 @@ const ( RunStatusExecuting RunStatus = "executing" RunStatusCompleted RunStatus = "completed" RunStatusFailed RunStatus = "failed" - RunStatusCancelled RunStatus = "cancelled" + RunStatusCanceled RunStatus = "canceled" RunStatusExpired RunStatus = "expired" + + // Deprecated: use RunStatusCanceled. Kept so existing callers compile; it + // writes the new spelling. + RunStatusCancelled = RunStatusCanceled + + // runStatusLegacyCancelled is the spelling rows carried before #554. It is + // read, never written. + runStatusLegacyCancelled RunStatus = "cancelled" ) // Validate returns an error when s is not a recognized RunStatus. @@ -26,31 +34,35 @@ func (s RunStatus) Validate() error { switch s { case RunStatusPlanned, RunStatusAwaitingApproval, RunStatusApproved, RunStatusExecuting, RunStatusCompleted, RunStatusFailed, - RunStatusCancelled, RunStatusExpired: + RunStatusCanceled, runStatusLegacyCancelled, RunStatusExpired: return nil } return fmt.Errorf("unknown run status %q", s) } // IsTerminal reports whether s is a terminal run status: RunStatusCompleted, -// RunStatusFailed, RunStatusCancelled, or RunStatusExpired. Terminal runs are +// RunStatusFailed, RunStatusCanceled (either spelling), or RunStatusExpired. Terminal runs are // the only ones allowed to carry a CompletedAt timestamp (see // RunRecord.Validate). func (s RunStatus) IsTerminal() bool { switch s { - case RunStatusCompleted, RunStatusFailed, RunStatusCancelled, RunStatusExpired: + case RunStatusCompleted, RunStatusFailed, RunStatusCanceled, runStatusLegacyCancelled, RunStatusExpired: return true } return false } // ParseRunStatus converts s into a RunStatus, returning a descriptive error -// when s is not a recognized value. +// when s is not a recognized value. The legacy "cancelled" reads as +// RunStatusCanceled. func ParseRunStatus(s string) (RunStatus, error) { st := RunStatus(s) if err := st.Validate(); err != nil { return "", err } + if st == runStatusLegacyCancelled { + return RunStatusCanceled, nil + } return st, nil } @@ -61,15 +73,23 @@ const ( TrancheStatusScheduled TrancheStatus = "scheduled" TrancheStatusFired TrancheStatus = "fired" TrancheStatusCompleted TrancheStatus = "completed" - TrancheStatusCancelled TrancheStatus = "cancelled" + TrancheStatusCanceled TrancheStatus = "canceled" TrancheStatusFailed TrancheStatus = "failed" + + // Deprecated: use TrancheStatusCanceled. Kept so existing callers compile; + // it writes the new spelling. + TrancheStatusCancelled = TrancheStatusCanceled + + // trancheStatusLegacyCancelled is the spelling rows carried before #554. + // It is read, never written. + trancheStatusLegacyCancelled TrancheStatus = "cancelled" ) // Validate returns an error when s is not a recognized TrancheStatus. func (s TrancheStatus) Validate() error { switch s { case TrancheStatusScheduled, TrancheStatusFired, TrancheStatusCompleted, - TrancheStatusCancelled, TrancheStatusFailed: + TrancheStatusCanceled, trancheStatusLegacyCancelled, TrancheStatusFailed: return nil } return fmt.Errorf("unknown tranche status %q", s) @@ -88,12 +108,16 @@ func (s TrancheStatus) allowsFiredAt() bool { } // ParseTrancheStatus converts s into a TrancheStatus, returning a -// descriptive error when s is not a recognized value. +// descriptive error when s is not a recognized value. The legacy "cancelled" +// reads as TrancheStatusCanceled. func ParseTrancheStatus(s string) (TrancheStatus, error) { st := TrancheStatus(s) if err := st.Validate(); err != nil { return "", err } + if st == trancheStatusLegacyCancelled { + return TrancheStatusCanceled, nil + } return st, nil } diff --git a/pkg/ladder/store_test.go b/pkg/ladder/store_test.go index d039a6fc..d0111dbd 100644 --- a/pkg/ladder/store_test.go +++ b/pkg/ladder/store_test.go @@ -19,7 +19,7 @@ func TestRunStatusIsTerminal(t *testing.T) { {RunStatusExecuting, false}, {RunStatusCompleted, true}, {RunStatusFailed, true}, - {RunStatusCancelled, true}, + {RunStatusCanceled, true}, {RunStatusExpired, true}, } for _, c := range cases { @@ -86,9 +86,9 @@ func TestRunRecordValidate(t *testing.T) { wantErr: false, }, { - name: "RunStatusCancelled with CompletedAt is valid", + name: "RunStatusCanceled with CompletedAt is valid", mutate: func(r *RunRecord) { - r.Status = RunStatusCancelled + r.Status = RunStatusCanceled r.CompletedAt = &now }, wantErr: false, @@ -229,9 +229,9 @@ func TestTrancheValidate(t *testing.T) { wantErr: true, }, { - name: "TrancheStatusCancelled with FiredAt is invalid", + name: "TrancheStatusCanceled with FiredAt is invalid", mutate: func(tr *Tranche) { - tr.Status = TrancheStatusCancelled + tr.Status = TrancheStatusCanceled tr.FiredAt = &now }, wantErr: true, diff --git a/pkg/ladder/types_test.go b/pkg/ladder/types_test.go index 9aa145e4..4632ccc1 100644 --- a/pkg/ladder/types_test.go +++ b/pkg/ladder/types_test.go @@ -175,7 +175,7 @@ func TestRunStatusValidate(t *testing.T) { {RunStatusExecuting, false}, {RunStatusCompleted, false}, {RunStatusFailed, false}, - {RunStatusCancelled, false}, + {RunStatusCanceled, false}, {RunStatusExpired, false}, {"unknown-status", true}, {"", true}, @@ -200,7 +200,7 @@ func TestTrancheStatusValidate(t *testing.T) { {TrancheStatusScheduled, false}, {TrancheStatusFired, false}, {TrancheStatusCompleted, false}, - {TrancheStatusCancelled, false}, + {TrancheStatusCanceled, false}, {TrancheStatusFailed, false}, {"unknown-tranche-status", true}, {"", true}, @@ -490,7 +490,8 @@ func TestParseRunStatus(t *testing.T) { {"executing", RunStatusExecuting, false}, {"completed", RunStatusCompleted, false}, {"failed", RunStatusFailed, false}, - {"cancelled", RunStatusCancelled, false}, + {"canceled", RunStatusCanceled, false}, + {"cancelled", RunStatusCanceled, false}, // legacy rows read as canceled (#554) {"expired", RunStatusExpired, false}, {"bogus", "", true}, {"", "", true}, @@ -523,7 +524,8 @@ func TestParseTrancheStatus(t *testing.T) { {"scheduled", TrancheStatusScheduled, false}, {"fired", TrancheStatusFired, false}, {"completed", TrancheStatusCompleted, false}, - {"cancelled", TrancheStatusCancelled, false}, + {"canceled", TrancheStatusCanceled, false}, + {"cancelled", TrancheStatusCanceled, false}, // legacy rows read as canceled (#554) {"failed", TrancheStatusFailed, false}, {"bogus", "", true}, {"", "", true},