Fix flaky smoketest due to race condition in subscribe #1656
Conversation
…ert the always-wait behavior of smoketest from #1536
bfops
force-pushed
the
noa/flaky-smoketest-subscribe
branch
from
13975d1 to
fe7f18e
Compare
| @@ -72,7 +72,7 @@ def test_add_then_remove_index(self): | |||
| # There are no indices, resulting in an unsupported unindexed join. | |||
| self.publish_module(name, clear = False) | |||
| with self.assertRaises(Exception): | |||
| self.subscribe(self.JOIN_QUERY, n = 0)() | |||
| self.subscribe(self.JOIN_QUERY, n = 0) | |||
There was a problem hiding this comment.
(if we've fixed the race condition, these changes from #1536 are no longer necessary)
smoketests/__init__.py
Outdated
| code = proc.poll() | ||
| if code: | ||
| raise subprocess.CalledProcessError(code, fake_args) | ||
| print("no inital update, but no error code either") |
There was a problem hiding this comment.
to double-check - is it intended that we trigger this both if the code is None and if the code is 0?
There was a problem hiding this comment.
Correct. Maybe it should check for 0, but it doesn't make much of a difference anyway.
There was a problem hiding this comment.
fair, I guess it just falls through to a thread that immediately returns below
There was a problem hiding this comment.
Oh - it should wait. I see the potential race condition here.
There was a problem hiding this comment.
Yeah that seems right. I suspect it's basically when the process hasn't quite exited yet, but I was unable to reproduce locally so I couldn't really narrow done.
|
Ran it |
This should fix it - now it'll only error if the initial update was never printed, i.e. the subscription never started. I'm about 90% sure that this should fix any race condition issues, but if there's any flakiness we can just go back to the fix from before.