Ignore three timing-dependent tests on Windows CI (APP-5820) - #15823
Open
warp-factories[bot] wants to merge 1 commit into
Open
Ignore three timing-dependent tests on Windows CI (APP-5820)#15823warp-factories[bot] wants to merge 1 commit into
warp-factories[bot] wants to merge 1 commit into
Conversation
Mark the three tests identified in APP-5820 as ignored on Windows only, matching the existing #[cfg_attr(windows, ignore = "...")] convention used elsewhere in this codebase for known Windows CI flakes. Coverage is preserved on Linux and macOS, where these tests are reliable. - terminal::shared_session::sharer::network::tests::test_handle_pty_read_event_while_batching - terminal::shared_session::sharer::network::tests::test_selection_updates_throttled_and_duplicates_ignored - terminal_session_view::tests::shell_completion_source_warmup_loads_path_executables This is the immediate mitigation requested in APP-5820 so that unrelated PRs stop getting blocked by these flakes. The real fix (replacing the implicit timing assumptions with polling on observable state and then removing these ignore attributes) is tracked separately in that issue.
Contributor
Author
|
This PR was generated with Warp. Comment |
Contributor
|
@warp-agent-staging[bot] I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
Contributor
There was a problem hiding this comment.
Overview
This PR conditionally ignores three existing timing-dependent tests on Windows while keeping them active on Linux and macOS.
Concerns
- No blocking concerns found. The added
cfg_attr(windows, ignore = ...)attributes match existing repository convention and include the tracking issue.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
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.



Description
Three timing-dependent tests have been flaking on Windows CI, each on a separate PR unrelated to the code they touch (see APP-5820):
terminal::shared_session::sharer::network::tests::test_handle_pty_read_event_while_batchingterminal::shared_session::sharer::network::tests::test_selection_updates_throttled_and_duplicates_ignoredterminal_session_view::tests::shell_completion_source_warmup_loads_path_executablesThis PR implements the "Immediate action" requested in the issue: mark these three tests
#[cfg_attr(windows, ignore = "APP-5820: flaky on Windows CI")], matching the existing convention already used in this codebase for known Windows-only flakes (e.g.TODO(CORE-3626)inhistory_tests.rs,input_tests.rs,completer/test.rs, etc.). Coverage is preserved on Linux and macOS, where these tests are reliable.The "Real fix" described in the issue (replacing the implicit timing assumptions with polling on observable state, then removing these ignore attributes) is intentionally out of scope for this PR and remains tracked on the issue.
Linked Issue
APP-5820
Testing
This is a test-only change (adding
#[cfg_attr(windows, ignore = ...)]attributes). No new tests are added; the change only affects which platforms run the three existing tests. Verified the edited files still parse correctly withrustc(fullcargo test/cargo checkwere not run in this environment).Agent Mode
CHANGELOG-NONE