Remove a flaky got_timeout assert from two channel tests#153534
Remove a flaky got_timeout assert from two channel tests#153534rust-bors[bot] merged 1 commit intorust-lang:mainfrom
got_timeout assert from two channel tests#153534Conversation
In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs.
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
| }); | ||
|
|
||
| let mut recv_count = 0; | ||
| let mut got_timeout = false; |
There was a problem hiding this comment.
I wonder why not just make the sender thread have a while loop on a shared atomic value (like an Arc<AtomicBool>) that yields itself through thread::yield_now? Or I'm wondering if we could also use a shared Condvar that waits in the sender thread and the receiver thread is the one that sends a notification? That way we can guarantee that our receiver thread experiences a RecvTimeoutError::Timeout error after 10 ms and either set the atomic value to a state that the while loop from the sender thread will stop looping on or the sender thread just resumes its iteration in the for loop upon receiving a notification.
|
It's not very clear to me what the intent of the tests here was. They were added in #33748 with the original recv_timeout impl, but I don't see any comments there on why we thought this test was particularly useful. I think I'd also be happy to r+ a full removal, but just removing the expectation of seeing a timeout seems OK for now. We do have a couple tests that verify recv_timeout does in fact timeout (though mostly with empty channels...), so I think we'll catch at least that kind of bug regardless. @bors r+ |
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
…uwer Rollup of 7 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #154094 (add neon load/store assembly test) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move])
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
Rollup of 6 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move])
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
Rollup of 7 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move]) - #154201 (Use enums to clarify `DepNodeColorMap` color marking )
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
Rollup of 9 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #153582 (Simplify find_attr! for HirId usage) - #154174 (allow `incomplete_features` in most UI tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move]) - #154201 (Use enums to clarify `DepNodeColorMap` color marking )
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
…lacrum Remove a flaky `got_timeout` assert from two channel tests In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs. One of these tests actually failed in rust-lang#153387 (comment). Earlier failures: - rust-lang#150365 (comment) - rust-lang#147775 (comment) --- - Prior art: rust-lang#152878
…uwer Rollup of 22 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #152543 (privacy: Fix type privacy holes in RPITITs) - #153107 (Optimize BTreeMap::append() using CursorMut) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #153718 (Fix environ on FreeBSD with cdylib targets that use -Wl,--no-undefined .) - #153857 (Rename `target.abi` to `target.cfg_abi` and enum-ify llvm_abiname) - #153880 (Lifted intersperse and intersperse_with Fused transformation and updated documentation + tests) - #153931 (remove usages of to-be-deprecated numeric constants) - #150630 (Unknown -> Unsupported compression algorithm) - #153491 (Move `freeze_*` methods to `OpenOptionsExt2`) - #153582 (Simplify find_attr! for HirId usage) - #153623 (std: move `sys::pal::os` to `sys::paths`) - #153647 (docs(fs): Clarify That File::lock Coordinates Across Processes) - #153936 (Skip stack_start_aligned for immediate-abort) - #154011 (implement `BinaryHeap::as_mut_slice`) - #154167 (ui/lto: move and rename two tests from issues/) - #154174 (allow `incomplete_features` in most UI tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move]) - #154201 (Use enums to clarify `DepNodeColorMap` color marking )
…uwer Rollup of 22 pull requests Successful merges: - #122668 (Add APIs for dealing with titlecase) - #152543 (privacy: Fix type privacy holes in RPITITs) - #153107 (Optimize BTreeMap::append() using CursorMut) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #153718 (Fix environ on FreeBSD with cdylib targets that use -Wl,--no-undefined .) - #153857 (Rename `target.abi` to `target.cfg_abi` and enum-ify llvm_abiname) - #153880 (Lifted intersperse and intersperse_with Fused transformation and updated documentation + tests) - #153931 (remove usages of to-be-deprecated numeric constants) - #150630 (Unknown -> Unsupported compression algorithm) - #153491 (Move `freeze_*` methods to `OpenOptionsExt2`) - #153582 (Simplify find_attr! for HirId usage) - #153623 (std: move `sys::pal::os` to `sys::paths`) - #153647 (docs(fs): Clarify That File::lock Coordinates Across Processes) - #153936 (Skip stack_start_aligned for immediate-abort) - #154011 (implement `BinaryHeap::as_mut_slice`) - #154167 (ui/lto: move and rename two tests from issues/) - #154174 (allow `incomplete_features` in most UI tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move]) - #154201 (Use enums to clarify `DepNodeColorMap` color marking )
…uwer Rollup of 21 pull requests Successful merges: - #152543 (privacy: Fix type privacy holes in RPITITs) - #153107 (Optimize BTreeMap::append() using CursorMut) - #153312 (Packages as namespaces part 1) - #153534 (Remove a flaky `got_timeout` assert from two channel tests) - #153718 (Fix environ on FreeBSD with cdylib targets that use -Wl,--no-undefined .) - #153857 (Rename `target.abi` to `target.cfg_abi` and enum-ify llvm_abiname) - #153880 (Lifted intersperse and intersperse_with Fused transformation and updated documentation + tests) - #153931 (remove usages of to-be-deprecated numeric constants) - #150630 (Unknown -> Unsupported compression algorithm) - #153491 (Move `freeze_*` methods to `OpenOptionsExt2`) - #153582 (Simplify find_attr! for HirId usage) - #153623 (std: move `sys::pal::os` to `sys::paths`) - #153647 (docs(fs): Clarify That File::lock Coordinates Across Processes) - #153936 (Skip stack_start_aligned for immediate-abort) - #154011 (implement `BinaryHeap::as_mut_slice`) - #154167 (ui/lto: move and rename two tests from issues/) - #154174 (allow `incomplete_features` in most UI tests) - #154175 (Add new alias for Guillaume Gomez email address) - #154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - #154188 (Update the tracking issue for #[diagnostic::on_move]) - #154201 (Use enums to clarify `DepNodeColorMap` color marking )
…uwer Rollup of 21 pull requests Successful merges: - rust-lang/rust#152543 (privacy: Fix type privacy holes in RPITITs) - rust-lang/rust#153107 (Optimize BTreeMap::append() using CursorMut) - rust-lang/rust#153312 (Packages as namespaces part 1) - rust-lang/rust#153534 (Remove a flaky `got_timeout` assert from two channel tests) - rust-lang/rust#153718 (Fix environ on FreeBSD with cdylib targets that use -Wl,--no-undefined .) - rust-lang/rust#153857 (Rename `target.abi` to `target.cfg_abi` and enum-ify llvm_abiname) - rust-lang/rust#153880 (Lifted intersperse and intersperse_with Fused transformation and updated documentation + tests) - rust-lang/rust#153931 (remove usages of to-be-deprecated numeric constants) - rust-lang/rust#150630 (Unknown -> Unsupported compression algorithm) - rust-lang/rust#153491 (Move `freeze_*` methods to `OpenOptionsExt2`) - rust-lang/rust#153582 (Simplify find_attr! for HirId usage) - rust-lang/rust#153623 (std: move `sys::pal::os` to `sys::paths`) - rust-lang/rust#153647 (docs(fs): Clarify That File::lock Coordinates Across Processes) - rust-lang/rust#153936 (Skip stack_start_aligned for immediate-abort) - rust-lang/rust#154011 (implement `BinaryHeap::as_mut_slice`) - rust-lang/rust#154167 (ui/lto: move and rename two tests from issues/) - rust-lang/rust#154174 (allow `incomplete_features` in most UI tests) - rust-lang/rust#154175 (Add new alias for Guillaume Gomez email address) - rust-lang/rust#154182 (diagnostics: avoid ICE for undeclared generic parameter in impl) - rust-lang/rust#154188 (Update the tracking issue for #[diagnostic::on_move]) - rust-lang/rust#154201 (Use enums to clarify `DepNodeColorMap` color marking )
In CI, the receiver thread can be descheduled for a surprisingly long time, so there's no guarantee that a timeout actually occurs.
One of these tests actually failed in #153387 (comment).
Earlier failures:
oneshottests #152878