search

typelevel / weaver-test

A test framework that runs everything in parallel.
https://typelevel.org/weaver-test/
Other
46 stars 8 forks source link

Memoised resources test is flaky #102

Open zainab-ali opened 2 weeks ago

zainab-ali commented 2 weeks ago

This issue was copied over from: https://github.com/disneystreaming/weaver-test/issues/475 It was opened by: keynmol

Not sure if it pops up more often because we're going through more SS updates or what.

[info] weaver.framework.test.MemoisedResourceTests
[error] - Memoised resources should be: 1s
[error]    * lazily allocated,
[error]    * shared when accessed concurrently
[error]    * not finalised until all uses are finished
[error]    * re-allocated on demand after being finalised
[error]  [0] assertion failed (modules/framework/cats/test/src/MemoisedResourceTests.scala:33)
[error]  [0] 
[error]  [0] } yield expect.all(initCount == 3, finCount == 3, useCount == 12)
[error]  [0]                    |         |
[error]  [0]                    4         false
[error] 
[error]  [1] assertion failed (modules/framework/cats/test/src/MemoisedResourceTests.scala:33)
[error]  [1] 
[error]  [1] } yield expect.all(initCount == 3, finCount == 3, useCount == 12)
[error]  [1]                                    |        |
[error]  [1]                                    4        false
[error] Failed: Total 41, Failed 1, Errors 0, Passed 40
[error] Failed tests:
[error]     weaver.framework.test.MemoisedResourceTests
[error] (cats2_12 / Test / test) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 13 s, completed Feb 9, 2022 11:22:14 PM

Sample build: https://github.com/disneystreaming/weaver-test/runs/5133422123?check_suite_focus=true Another one: https://github.com/disneystreaming/weaver-test/runs/4781690405?check_suite_focus=true

zainab-ali commented 2 weeks ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/475#issuecomment-1040050982 It was written by: Baccata

Now that CE3 has some proper test kit, we should be using it whenever relevant ... but that implies dropping CE2. Which I'm happy to do at this time.

zainab-ali commented 2 weeks ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/475#issuecomment-1698143419 It was written by: mcanlas

I wonder if I ran into this feature. I'm working on a codebase where we are using putLazyR (not for any specific reason I don't think) and it initialized the resource twice. Yet when I use putLazyR in a contrived sandbox using the code in the docs, it initializes once. Potential race condition with this feature???

zainab-ali commented 2 weeks ago

This comment was copied over from: https://github.com/disneystreaming/weaver-test/issues/475#issuecomment-1700471030 It was written by: Baccata

mcanlas the Memoized resource that backs the putLazyR is essentially meant to ensure that there's only a live one at any given time. This helps maximising available system resources when you have huge test benches with lots and lots of suites.

In the situation where a suite that uses the memoized resource would finish before a second suite that uses the same memoized resource starts, the resource would clean up after the first suite ends, and be re-initialised after the second suite starts. We cannot guarantee that it won't happen, because weaver has zero control over how many suites are run in parallel, that decision belongs to the build tool.

The reason this why this test is flaky is that it relies on actual sleeps, which makes any suite inherently flaky if they don't use CE's TestControl construct.