HIVE-29842: fixing flaky TestYarnQueueMetricsCollector tests - #6729
HIVE-29842: fixing flaky TestYarnQueueMetricsCollector tests#6729konstantinb wants to merge 2 commits into
Conversation
|
|
@abstractdog @architjainjain could you take a look when you have a chance? This fixes the flaky Since this is a flaky-test fix: could a committer trigger a hive-flaky-check run for this branch as supporting evidence? I can view the job but don't have build permission. Parameters: |



What changes were proposed in this pull request?
HIVE-29842: Test-only changes to
TestYarnQueueMetricsCollector(no production code touched):awaitFirstPoll()helper:waitForInvocationCount+ count assert, the same idiom the class already uses) in the three tests whose stubs are consumed only by the async refresh thread, before asserting and shutting down.throws Exceptionclauses only those stubs required): they either duplicate the lenient@Beforedefaults verbatim or set values the test's assertion never reads.setupHappyPathMocks()javadoc: a strict per-test stub consumed only by the refresh thread must be awaited before shutdown.Why are the changes needed?
Four tests stub interactions whose only consumer is the async refresh task (scheduled with
scheduleWithFixedDelay(task, 0, ...)), then assert and callshutdown()without awaiting the poll. Whenshutdown()wins that race, the strict stub is never consumed andMockitoJUnitRunnerfails the whole class withUnnecessaryStubbingException— this hit the precommit of an unrelated PR (run 6 of PR 6676; see HIVE-29842 for the link).The tests usually pass only by accident: the collector's INFO logging during scheduling stalls the main thread long enough for the poll thread to sneak its call in. That makes the flake deterministic to reproduce on master:
(suppressing logging removes the log-I/O stall; the class then fails consistently pre-patch).
The awaits also make the assertions meaningful: previously
assertNull(getLatestSnapshot())passed whether the stubbed condition was exercised or the poll simply had not run yet.Does this PR introduce any user-facing change?
No. Test-only.
How was this patch tested?