Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaky Tests #82

Open
MateuszKowalewski opened this issue Jun 29, 2023 · 5 comments
Open

Flaky Tests #82

MateuszKowalewski opened this issue Jun 29, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@MateuszKowalewski
Copy link
Contributor

I've experienced some flakynes in some tests. There seems something wrong with the current runtime.

Here's a zip with screenshots:
Screenshot_2023062.zip

A heuristic of my test failures would look something like this:

delayed injectL		|||||
blocking		|||||||||||||||||||||||||||
mergeAll		||||||||||||||||
LList.sortBySignal	||
merge			|||||
latestValue			
splitResourceAsync	|

Test failures were triggered quite reliably by calling clean ; compile ; test in SBT, especially when there was additional load on the machine, or when called in short sequence.

Maybe just the timeouts are too short. But they are actually quite long for what is computed. So this should just not happen, I guess.

But I didn't investigate further until now. So can't say much besides that it happens at the moment.

@TomasMikula
Copy link
Owner

Thanks for reporting! I have definitely encountered the first 3, don't remember about the others. delayed injectL is in my estimate the one that I hit overwhelmingly most often on my machine.

Most of these tests are "broken by design": they rely on some scheduling guarantees which should not be assumed.

blocking, I think, also suffers from the "non-monotonicity" of time measurements across threads:

  1. Get current time t0 from one thread.
  2. sleep for 10ms (as per some built-in scheduler)
  3. Get current time t1 from some other thread.
  4. t1 - t0 may be less than 10ms.

Flaky tests are definitely something that needs to be fixed. Though maybe not all at once.

@TomasMikula TomasMikula added the bug Something isn't working label Jul 1, 2023
@MateuszKowalewski
Copy link
Contributor Author

Improving and hardening the runtime should fix this I guess.

It's not like the test as such would have issues. It's how they are executed.

Testing async runtimes is trick anyway in itself…

@TomasMikula
Copy link
Owner

I have rewritten delayed injectL test in a deterministic fashion. It should now never fail. If it does, please, re-report.

@MateuszKowalewski
Copy link
Contributor Author

Thanks a lot!

But does it make sense to heal symptoms of an underlying problem?

If the current runtime has issues with timeouts that should not go unnoticed. Just "tuning" tests so these issues don't show up defeats the idea behind tests, imho. That's like: "I've made all test pass by deleting them…" 😉

I would in this case just document the behavior so it's not unexpected, and look into the root cause.

If looking into the root cause is currently out of scope, that's also fine. Just create a ticket and look into that when time has come.

Never mind, just my 2ct.

@TomasMikula
Copy link
Owner

The delayed injectL test was actually broken:

It was doing

clock.advanceBy(31.millis)

but if that adjustment of time happened before the tested program did

delay(30.millis)

the test would inevitably timeout, because the clock will not be advanced by another 30 millis for this delay to finish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants