Remove timing assertion from oneshot::send_before_recv_timeout

[JonathanBrouwer]

This test regularly spuriously fails in CI, such as #152632 (comment)
We can just remove the assertion but I'd like to understand why, so I'm adding more information to the assert

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 8 candidates
• Random selection from Mark-Simulacrum, jhpratt, joboet

[jieyouxu]

Is there anything we can do for this test to make it less likely to spuriously fail? (Not really for this PR, but yeah I've seen it fail quite often.)

[JonathanBrouwer]

I don't know fully how these tests work, my guess is that all the tests run in parallel threads and this thread yields and just isn't scheduled for a while., therefore hitting the timeout. I wanted to increase the timeout to fix that but this PR is adding more information to know what to increase the timeout to.

We can also just remove the assertion if we want to be done with it, but that doesn't feel right

[jhpratt]

Can't hurt to have a bit of instrumentation to narrow things down. Prioritizing given the spuriousness of it.

@bors r+ rollup p=1

[rust-bors]

📌 Commit d5a1a7a has been approved by jhpratt

It is now in the queue for this repository.

[Zalathar]

In #152145, a similar test oneshot::recv_timeout_before_send was disabled for being inherently flaky, because there was no guarantee that the sender thread would actually send before the receiver thread timed out.

This test also appears to be inherently flaky, for a related but sillier reason.

Note that the failing assertion is the final line, assert!(start.elapsed() < timeout). Even if the receive succeeds (which it should), there's no guarantee that the assertion is performed before 1 second is elapsed.

All it takes is for that assertion to fail is for the CI runner to be descheduled for 1 second or longer, at any point after the initial let start = Instant::now().

[JonathanBrouwer]

Ah, I was thinking something similar but I was like "there is no way a thread isn't scheduled for a full second".
Given these other tests I think it is indeed best just to ignore the test

[rust-bors]

⚠️ A new commit cbe63ddd9a0ddedb3b32b902ad0dc39db1593144 was pushed to the branch, the PR will need to be re-approved.

[JonathanBrouwer]

I disabled the test, similar to the other PR

[JonathanBrouwer]

Hmmm pushing a new commit does not unapprove the rollup, interesting

[Zalathar]

In this case, I believe we should be able to keep the test and only get rid of the timing assertion.

[JonathanBrouwer]

Fixed :)