Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The three narrowly scoped test changes prevent unintended day-boundary splits without changing assertions or production code.
No open issues 🎉
What changed in this PR
This PR extends the clock-pinning approach from #924 to three notification controller tests, preventing midnight failures caused by UTC-day follower grouping.
Changes:
- Pins two web controller tests and one API controller test to noon.
- Keeps assertions and production behavior unchanged.
| File | Description |
|---|---|
| tests/Feature/Controllers/NotificationControllerTest.php | Pins the follow-back and actor-lane tests to noon. |
| tests/Feature/Controllers/Api/NotificationControllerTest.php | Pins the notification group metadata test to noon. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow notifications are grouped per UTC day, so two follows a minute apart land in separate groups when midnight falls between them. #924 pinned the clock for one test that relied on them being grouped, but three more do the same:
NotificationControllerTest: "says whether the user follows each new follower back"NotificationControllerTest: "gives the lane of every actor so each notification takes their color"Api\NotificationControllerTest: "says which group a notification leads, how many events it has, and whether the user follows the actor"Each creates one follow a minute or two before another and expects a single group, so a CI run in the first couple of minutes after 00:00 UTC fails. Like #924, they now start at noon with
$this->travelTo(today()->setTime(12, 0)). Nothing changes outside the tests.To find them, I ran the whole suite with the clock frozen at 00:00:30, 00:01:30 and 12:00 UTC. These three were the only ones that failed only just after midnight. Before the change, 3 failed at 00:00:30 and 1 at 00:01:30. After it, all three pass at every time, and so do the other notification tests (105 in all), Pint and Rector.