Skip to content

Fixed flaky comments test by pinning created_at#27434

Merged
EvanHahn merged 1 commit into
mainfrom
fix-flaky-test-with-comment-ordering
Apr 16, 2026
Merged

Fixed flaky comments test by pinning created_at#27434
EvanHahn merged 1 commit into
mainfrom
fix-flaky-test-with-comment-ordering

Conversation

@EvanHahn
Copy link
Copy Markdown
Contributor

@EvanHahn EvanHahn commented Apr 16, 2026

no ref

This is a test-only change.

I think Claude Opus 4.7's explanation is reasonably good:

The test at comments.test.js:1362 uses dbFns.addCommentWithReplies to create a parent comment and a reply back-to-back without explicit created_at values. Both rows rely on the model's default timestamp (new Date() at save time).

The admin browseAll controller at comments-controller.js:166 sorts by created_at desc and the SQL has no secondary tiebreak. When the two inserts land in the same millisecond, MySQL happens to return rows in insertion order (parent first, reply second), which matches the stored snapshot [commentMatcher, commentMatcherWithParent]. When the reply spills into a newer millisecond, desc puts the reply first — the snapshot then sees parent (reply's parent object) missing from index 1 and the assertion fails exactly as in the CI log.

- `Returns flat list including replies by default` occasionally failed on CI because the parent and reply created by `dbFns.addCommentWithReplies` relied on the model's default `created_at` (`new Date()`).
- The admin `browseAll` controller orders by `created_at desc` with no secondary tiebreak, so when both rows landed in the same millisecond MySQL returned them in insertion order (matching the snapshot), but when the reply spilled into a newer millisecond it came first and the snapshot broke.
- Pinned explicit `created_at` values so the reply is deterministically newer than the parent, swapped the expected matcher array, and updated the snapshot to match the natural desc order.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

Walkthrough

The test file for the comments API endpoint is updated to include explicit created_at timestamps for test comment data. The expected response snapshot is adjusted to reflect the resulting sort order, where the reply comment appears before the parent comment based on descending timestamp ordering. No public APIs or exported declarations are modified by these test-only changes.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing a flaky test by pinning the created_at timestamps for deterministic test ordering.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly explains the test-only changes, the root cause of the flakiness, and how the fix addresses it.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-flaky-test-with-comment-ordering

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@EvanHahn EvanHahn marked this pull request as ready for review April 16, 2026 16:35
@EvanHahn EvanHahn changed the title Fixed flaky admin comments test by pinning created_at Fixed flaky comments test by pinning created_at Apr 16, 2026
@EvanHahn EvanHahn enabled auto-merge (squash) April 16, 2026 16:36
@sonarqubecloud
Copy link
Copy Markdown

@EvanHahn EvanHahn disabled auto-merge April 16, 2026 16:37
@EvanHahn EvanHahn enabled auto-merge (squash) April 16, 2026 16:37
@EvanHahn EvanHahn merged commit 1150f0a into main Apr 16, 2026
44 checks passed
@EvanHahn EvanHahn deleted the fix-flaky-test-with-comment-ordering branch April 16, 2026 16:56
franky19 pushed a commit to franky19/Ghost that referenced this pull request Apr 18, 2026
no ref

This is a test-only change.

I think Claude Opus 4.7's explanation is reasonably good:

> The test at `comments.test.js:1362` uses `dbFns.addCommentWithReplies`
to create a parent comment and a reply back-to-back without explicit
`created_at` values. Both rows rely on the model's default timestamp
(`new Date()` at save time).
>
> The admin browseAll controller at `comments-controller.js:166` sorts
by `created_at desc` and the SQL has no secondary tiebreak. When the two
inserts land in the same millisecond, MySQL happens to return rows in
insertion order (parent first, reply second), which matches the stored
snapshot `[commentMatcher, commentMatcherWithParent]`. When the reply
spills into a newer millisecond, `desc` puts the reply first — the
snapshot then sees `parent` (reply's parent object) missing from index 1
and the assertion fails exactly as in the CI log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant