Always use a timeout calling Poll::poll in tests

[Thomasdezeeuw]

This is to prevent #1226 from happening too some extend, see the docs of NO_TIMEOUT in tests/util/mod.rs.

[dtacalau]

While I understand the problem this patch is trying to fix, I think the patch has its own problems:

poll(None) vs poll(timeout) exercise different code paths. Removing all poll(None) from tests means its code paths are no longer tested.
Having an assert(Poll(timeout)) failing doesn't seem more data than knowing a Poll(None) hangs.
Maybe the CI can be configured to terminate a test earlier than 1 hour?

[kleimkuhler]

@dtacalau Brings up a good point about testing the None case for Poll::poll. I'd like to see this change go in, but maybe add a simple test that still uses None and expect an event event that should reliably happen? It wouldn't need need to be anything that tests additional behavior.

[Thomasdezeeuw]

I agree with @dtacalau and been looking into other solutions. I found the --ensure-time flag which:

Treat excess of the test execution time limit as
error.
Threshold values for this option can be configured via
RUST_TEST_TIME_UNIT, RUST_TEST_TIME_INTEGRATION
and
RUST_TEST_TIME_DOCTEST environment variables.
Expected format of environment variable is
VARIABLE=WARN_TIME,CRITICAL_TIME.
CRITICAL_TIME here means the limit that should not
be exceeded by test.

See cargo test -- --help. But its a nightly compiler only thing, see rust-lang/rust#64873 and rust-lang/rust#64888. I haven't been able to get it to actually run with the flag set.

[Thomasdezeeuw]

Oh you need to pass -Zunstable-options. But still will only work on nightly only compilers.

[Thomasdezeeuw]

--ensure-time won't work at all as the test isn't interrupted, so we do need a change like this.

Or we could just use the timeout command.

[kleimkuhler]

@Thomasdezeeuw Unfortunately it doesn't look like there is a great alternative for that on Windows. It looks like it some kind of equivalent could be used if CI was setup to run through Powerhsell?

[Thomasdezeeuw]

Not doing it this way and I haven't found an alternative to timeout on Windows/macOS.

[Thomasdezeeuw]

Something the following could work in the future.

RUST_TEST_TIME_UNIT="5000,5000" RUST_TEST_TIME_INTEGRATION="5000,5000" RUST_TEST_TIME_DOCTEST="5000,5000" cargo test --all-features -- -Z unstable-options --ensure-time