spotify / heroic

The Heroic Time Series Database
https://spotify.github.io/heroic/
Apache License 2.0
848 stars 109 forks source link

Rework all timer-based Tests #735

Open sming opened 3 years ago

sming commented 3 years ago

Timer based tests are unreliable

Use Case Resolved: developers are wasting many hours chasing down non-deterministic Test failures

Design & Implementation Notes

  1. Find all offending tests :
    1. Search in IntelliJ for Thread.sleep(.*)
    2. Search in IntelliJ for all usages of retryUntilResolved
    3. Search in IntelliJ for all usages of eu.toolchain.async.RetryPolicy
  2. Systematically go through all the tests, identifying and fixing tests that decide to proceed in a non-deterministic way.

What is non-deterministic and what isn’t?

“Non-deterministic” probably isn’t the correct technical term, but it means a situation where a false-positive GO / Green Light / Proceed occurs.

An example. Consider com.spotify.heroic.test.AbstractSuggestBackendIT#checks. The “bad”, non-deterministic first iteration of it proceeded when any number of suggestions were returned :

…
            .directTransform(result -> {
                if (result.getSuggestions().isEmpty()) {
                    throw new IllegalStateException("No tag suggestion 
…

- this meant that if only a single suggestion was returned, the test incorrectly proceeded. The deterministic fix was to pass in the expected number of suggestions. Then you know for sure you can proceed.