uber-go / goleak

Goroutine leak detector

MIT License

4.55k stars 151 forks source link

Fix flaky tests #36

Closed prashantv closed 4 years ago

prashantv commented 5 years ago

Some tests assume that stack.All() will give a stable set of goroutines: that any background goroutines from the test framework/previous tests have run till they're blocked.

Add verification that stack.All() returns a "stable" set of stacks before running these tests.

codecov[bot] commented 5 years ago

Codecov Report

Merging #36 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   95.03%   95.03%           
=======================================
  Files           4        4           
  Lines         141      141           
=======================================
  Hits          134      134           
  Misses          4        4           
  Partials        3        3

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 227bd74...5254a20. Read the comment docs.

prashantv commented 5 years ago

Dealing with non-determinism of schedulers is not fun, basically had to add retries to the stack.All() call till we get a "stable" state.

abhinav commented 4 years ago

There's a fair amount of similarity between the logic between the two tests (besides the retry limit). Any reason not to share the logic?

prashantv commented 4 years ago

There's a fair amount of similarity between the logic between the two tests (besides the retry limit). Any reason not to share the logic?

Agree, wanted to share the logic but it requires a bunch of moving around code (new test-only package, making the tests a separate _test package, etc). The movement around didn't seem worth it so kept it simple.