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

Memoised resources test is flaky #102

Open
zainab-ali opened this issue Nov 7, 2024 · 3 comments
Open

Memoised resources test is flaky #102

zainab-ali opened this issue Nov 7, 2024 · 3 comments

Comments

@zainab-ali
Copy link
Contributor

This issue was copied over from: disneystreaming/weaver-test#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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#475 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#475 (comment)
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
Copy link
Contributor Author

This comment was copied over from: disneystreaming/weaver-test#475 (comment)
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant