[RSDK-9365] increase timeout on flaky test #372
Conversation
| @@ -1357,7 +1357,7 @@ mod tests { | |||
| let _task = cloned_exec.spawn(async move { | |||
| viam_server.run().await; | |||
| }); | |||
| let _ = Timer::after(Duration::from_millis(500)).await; | |||
| let _ = Timer::after(Duration::from_millis(1000)).await; | |||
There was a problem hiding this comment.
I don't see a way you can completely eliminate the possibility of a race between the _fake_server_task and the viam_server.run() task given what we have now, but you can I think significantly shrink the window. Put some sort of condvar like object into play, and have the app.auth_fn signal it when it. Then, wait on it before asserting on the state of cloned_ram_storage. That will at least guarantee that the auth endpoint on run_fake_app_server was invoked, meaning that the viam server has progressed far enough to have attempted it. You will still need a timeout, but I'll bet a much smaller one than 500 milliseconds will be effective in practice, rather than goign the other way and just doubling the timeout to one second.
A fully deterministic solution would probably require adding hooks into ViamServer to allow tests to wait for it to have progressed to certain will known states, and we don't have that now. We probably should at some point.
the test has occasionally tripped, poisoning the
global_network_lock(fixed).It's unclear how much time it would be needed to ensure it doesn't trip (or if other manageable factors are involved), but 1sec seems like a still-acceptable upper-bound on a single test. still faster than running the whole CI workflow again.
If other tests with similar timeouts have this issue it might make sense to make the timeout value global for tests.