tests: retry deferred test assertion in t.Cleanup#1095
Conversation
yhabteab
force-pushed
the
fix-object-sync-tests
branch
from
c85267d to
98ebe0e
Compare
oxzi
left a comment
oxzi
left a comment
There was a problem hiding this comment.
Sounds reasonable, thanks!
However, could you please add the same guard to the other function call as well?
icingadb/tests/object_sync_test.go
Line 322 in 98ebe0e
Sure! Though, it doesn't need such a guard, since that assertion is for the initial-config sync, and my PR didn't add any behavioral changes to the initial-config sync. But I can guard it if you think it's necessary for consistency. |
Just like all other test assertions, this one should also be retried until it succeeds or times out. It seems this wasn't a problem before, but with the upcoming changes to Icinga 2.16, this assertion will fail occasionally since the queries are now sent with a bit of delay. By retrying the assertion, we can ensure that it will eventually succeed once the queries are processed or time out after a reasonable period.
yhabteab
force-pushed
the
fix-object-sync-tests
branch
from
98ebe0e to
7497d4c
Compare
Done. |
2 participants
All the time, I was thinking that there must be some race condition in my Icinga/icinga2#10619 PR that causes the test to fail occasionally1 and almost gone crazy trying to figure out what it is. However, it turns out that the issue is simply that the assertion is being executed before the queries have been processed, which is why it fails occasionally. After adding the retry mechanism, not a single failure has occurred after running the test hundreds of times.
Footnotes
https://github.com/Icinga/icinga2/pull/10619#issuecomment-3998801275 ↩