shared/logs: deflake follow handoff ordering tests#972
shared/logs: deflake follow handoff ordering tests#972amirHdev wants to merge 2 commits intokubetail-org:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58680a8d2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
amirHdev
force-pushed
the
fix-shared-logs-follow-order-test-flake
branch
2 times, most recently
from
07b5c30 to
e746abf
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e746abf83b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- relax shared/logs follow tests to allow valid cross-source ordering at the past-to-follow handoff - accept the expected UNAVAILABLE shutdown status in cluster_agent watcher tests after cancellation instead of assuming try_recv() is always empty Signed-off-by: Amirhossein Akhlaghpour <m9.akhlaghpoor@gmail.com>
amirHdev
force-pushed
the
fix-shared-logs-follow-order-test-flake
branch
from
e746abf to
392604a
Compare
can we review it again ? |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
The Rust and Go changes are independent and should be split into separate PRs. In terms of the Rust code, which test/issue is this designed to fix? In terms of the Go code, the proposed changes are changing the tests for certain specific test cases (as @codex pointed out). If that is the best solution then those cases should be moved to their own separate functions but the PR description needs to fully document why the checks can be relaxed. Also here's our PR tempalte and suggested commit format: |
|
Codex couldn't complete this request. Try again later. |
2 participants
This PR fixes an intermittent test failure in
modules/shared/logsseen in PR checks where follow-mode records from different sources can arrive in either order around the past-to-follow handoff.Fixes #696
Changes
TestStreamAllWithFollowandTestStreamTailWithFollowto avoid assuming a strict cross-source ordering for concurrent follow recordss1-*/s2-*) in the affected handoff scenariosWhy
Validation
go test ./shared/logs -run 'TestStream(All|Tail)WithFollow' -count=50(frommodules/)