Fixed and re-enabled the batch-sending integration tests#28799
Conversation
no ref The suite was skipped in early 2025 for "intermittent" failures, but had since rotted into deterministic failure: the email-renderer template was rewritten (copyright year now 2026) so every snapshot was stale, and two Replacements tests hardcoded the unsubscribe-link port 2369, which the Mocha to Vitest migration made per-fork. Regenerated the snapshots and matched the port flexibly (the snapshots already scrub it); 42/42 pass, stable across repeated runs. A latent within-file ordering dependency remains (dormant, since CI preserves declaration order) and is worth hardening separately.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe integration test suite for email batch sending in 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration:no-coverage |
✅ Succeeded | 2m 29s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 18s | View ↗ |
nx run ghost:test:ci:legacy |
✅ Succeeded | 2m 43s | View ↗ |
nx run ghost:test:ci:e2e |
✅ Succeeded | 2m 34s | View ↗ |
nx run ghost:test:ci:e2e:no-coverage |
✅ Succeeded | 2m 14s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 27s | View ↗ |
nx run-many -t lint -p ghost |
✅ Succeeded | 43s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-22 19:29:56 UTC
no ref The email footer renders the copyright year live (email-renderer.js uses new Date().getFullYear()), and matchEmailSnapshot scrubbed the unsubscribe URL, redirect links, keys and uuids but not the year — so the regenerated snapshots would fail on the next January 1. Added a year scrub to the shared matcher (the current year, resolved from the same process clock the renderer uses, so it always matches what was rendered) and regenerated the batch-sending and cards snapshots. Fixed copyright years in content (e.g. 2024) are left intact.
no ref The suite seeds members once in beforeAll and several tests add more, cleaning up inline. When a test threw before its inline cleanup — e.g. the timing- sensitive "submitted" assertion in the first member-creating test, which fails on mysql — the leaked subscriber shifted every later test's recipient count, turning one failure into eleven. Snapshot the seeded member ids in beforeEach and drop any extras in afterEach so a single failure can't poison the rest, and removed the now-stale FLAKEY markers. Verified 4x on mysql and 2x on sqlite, 42/42 each.
1 participant
Re-enables
batch-sending.test.js(skipped since early 2025) and hardens it to pass on both engines.Re-enable
Skipped for "intermittent" failures; it had since rotted into deterministic failure:
Replacementstests pinned127.0.0.1:2369, but the migration derives a per-fork port. Matched with\d+.Year scrub
The footer renders the year live (
new Date().getFullYear()) andmatchEmailSnapshotdidn't scrub it — so the snapshots would rot every January. Added a year scrub to the shared matcher;cards.test.js.snapregenerated (year-only) as a result.Member-leak cascade (the mysql failures)
The suite seeds members once and several tests add more, cleaning up inline. When a test threw before its inline cleanup — e.g. the timing-sensitive
submittedassertion that fails on mysql — the leaked subscriber shifted every later test's recipient count, turning one failure into eleven (the CI red). NowbeforeEachsnapshots the seeded member ids andafterEachdrops any extras, so a single failure can't poison the rest. Removed the stale// FLAKEYmarkers.Verified 4× on mysql + 2× on sqlite, 42/42 each.