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: Several tests in AsyncTest fail when run individually #1065

Closed
sleberknight opened this issue Oct 9, 2023 · 1 comment · Fixed by #1066
Closed

Flaky tests: Several tests in AsyncTest fail when run individually #1065

sleberknight opened this issue Oct 9, 2023 · 1 comment · Fixed by #1066
Assignees
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Milestone

Comments

@sleberknight
Copy link
Member

In #1024 I made the AsyncTest execute faster by reducing timeouts and delays. However, when researching #732 I happened to execute some tests individually, as opposed to executing the entire AsyncTest or all tests in kiwi. One failed. I then executed every single test individually and found two more that consistently fail when run individually, but pass when you run all the tests or run all tests in AsyncTest. These tests are failing on my new 2023 M2 MacBook Pro, as well as my old-as-dirt 2012 MacBook Pro (yes, 2012).

However, they actually sometimes fail differently on the new vs. old laptop! On the 2023 M2, the assertThatThrownBy fails, with the message "Expecting code to raise a throwable" while on the 2012, the assertThatThrownBy sometimes does not fail, but the subsequent assertion on the task count does fail. Given how much faster 2023 is than the 2012, this is not really all that surprising, and these tests became more likely to fail when I made them faster (#1024) because I reduced the task delay from 100 to 10 milliseconds, and the timeouts from 5 milliseconds to 1. This reduces the margin of error, apparently enough to cause these tests to be flaky.

When run individually, I suspect that the threads are being started so quickly that the tasks (which now have a 10ms) are completing before the waitForXxx calls are made. And since the waitForXxx calls are made inside the assertThatThrownBy, there is additional overhead by AssertJ that exists when running the tests individually, that may have already been executed when running the entire test suite.

I'm not going to try and figure out the exact details of all this, but rather will make targeted changes to these three tests to increase the delays, which should ensure they pass when run individually or as part of the overall test or test suite.

The tests that fail individually are:

  • WaitFor#shouldThrowAsyncException_WhenTimesOut_BeforeTheFutureCompletes
  • WaitForAll#shouldThrowAsyncException_WhenTimesOut_BeforeAllFuturesComplete
  • WaitForAllIgnoringType#shouldThrowAsyncException_WhenTimesOut_BeforeAllFuturesComplete
@sleberknight sleberknight added the code cleanup Fix issues reported by Sonar or any other code analysis tools label Oct 9, 2023
@sleberknight sleberknight added this to the 3.1.1 milestone Oct 9, 2023
@sleberknight sleberknight self-assigned this Oct 9, 2023
@sleberknight
Copy link
Member Author

Note that the above three tests also fail when run individually in VSCode in Gitpod.

sleberknight added a commit that referenced this issue Oct 11, 2023
* Make the duration of the ConcurrentTask higher for the flaky
  tests so that the difference between the timeout (1 ms) and
  the task duration is much larger than 9 ms
* Change the Awaitility poll interval from the default (100 ms)
  to 25 ms, which speeds the tests up, especially the ones that
  are awaiting normal completion
* Make the flaky tests RetryingTests with up to three attempts
* For clarity of intent, rename the delay field in ConcurrentTask
  to duration, since it is simulating an async task of a specific
  duration, not delaying task execution.

Closes  #1065
dsingley pushed a commit that referenced this issue Oct 11, 2023
* Make the duration of the ConcurrentTask higher for the flaky
  tests so that the difference between the timeout (1 ms) and
  the task duration is much larger than 9 ms
* Change the Awaitility poll interval from the default (100 ms)
  to 25 ms, which speeds the tests up, especially the ones that
  are awaiting normal completion
* Make the flaky tests RetryingTests with up to three attempts
* For clarity of intent, rename the delay field in ConcurrentTask
  to duration, since it is simulating an async task of a specific
  duration, not delaying task execution.

Closes  #1065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Fix issues reported by Sonar or any other code analysis tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant