amadan
Make the seeding-failure test assert the failure happened
The branch review found it vacuous: it asserted EnsurePerson succeeded and List did not error, both of which hold just as well if seeding silently succeeded or was skipped entirely by a later refactor. It was named for a failure it never checked occurred — the same shape as TestSeedRequiresDevelopmentEnv and TestApplyUpdateDoesNotBumpAnother OwnersDocument before it. Now asserts fewer than len(seedDocs) documents survived, which is positive evidence the broken core was reached and the loop died partway. Verified by swapping the deliberately-closed core for a working one and watching it fail: "got 6 documents, want fewer than 6". Not asserting an exact count: where Seed dies is an implementation detail, that it died at all is the property under test.
1 file changed,
+17
−1
internal/docs/sample_documents_internal_test.go+17 −1
CI passed — run details
diff --git a/internal/docs/sample_documents_internal_test.go b/internal/docs/sample_documents_internal_test.go| index eaa0b9d..e2aa5cb 100644 |
| --- a/internal/docs/sample_documents_internal_test.go |
| +++ b/internal/docs/sample_documents_internal_test.go |
| @@ -151,9 +151,25 @@ func TestEnsurePersonSurvivesSeedingFailure(t *testing.T) { |
| // this asserts "does not break", not "produced zero documents", |
| // since Seed's per-document loop may have left one partial row |
| // behind before its first ApplyUpdate failed). |
| - if _, err := app.Store().List(ctx, id); err != nil { |
| + docs, err := app.Store().List(ctx, id) |
| + if err != nil { |
| t.Fatalf("List for the person seeding failed for: %v", err) |
| } |
| + |
| + // Assert the seeding actually FAILED. Without this the test is |
| + // vacuous: "EnsurePerson succeeded and List did not error" holds |
| + // just as well if seeding silently succeeded, or was skipped |
| + // because a future refactor stopped calling seedIfEnabled at all |
| + // — in which case this test would keep passing while proving |
| + // nothing about surviving a failure. A complete seed is |
| + // len(seedDocs) documents, so anything short of that is |
| + // positive evidence the broken core was reached and the loop died |
| + // partway. Deliberately not asserting an exact count: where |
| + // precisely Seed dies is an implementation detail, but that it |
| + // died at all is the whole property under test. |
| + if len(docs) >= len(seedDocs) { |
| + t.Fatalf("got %d documents, want fewer than %d — seeding was expected to FAIL against the closed core, so this test is not proving what it claims", len(docs), len(seedDocs)) |
| + } |
| } |
| // TestConcurrentFirstSignInsProduceOnePersonAndOneSampleSet is the |