Skip to content

Commit

Permalink
Fix FeedbackCollectorTest#testGatheringOfDataNoScreenshot
Browse files Browse the repository at this point in the history
This test was added in https://codereview.chromium.org/2897323002/ but was
making incorrect assets. That is, when we're not expecting screeshots, the
results from the connectivity task should trigger the test callback.

It was passing locally because of timing. The things we were checking for in
the tests would be set once FeedbackCollector#maybePostResult would post task a
call to the callback in the test. The incorrect assert would be made before the
callback would get called.

TBR=nyquist@chromium.org
BUG=726704

Review-Url: https://codereview.chromium.org/2906903002
Cr-Commit-Position: refs/heads/master@{#475015}
  • Loading branch information
ymalik authored and Commit bot committed May 26, 2017
1 parent afc52f4 commit ac57656
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ public void run() {
}

private boolean shouldWaitForScreenshot() {
// We should always wait for the screenshot unless we're not taking one.
return mTakeScreenshot && !mScreenshotTaskFinished;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,13 +367,8 @@ public void onResult(FeedbackCollector collector) {
Assert.assertFalse("Result should not be ready directly after creation.", hasResult.get());
ConnectivityTask.FeedbackData feedbackData = createFeedbackData();
mCollector.onResult(feedbackData);
Assert.assertFalse("Result should not be ready after connectivity data.", hasResult.get());

// This timeout task should trigger the callback since we shouldn't wait for the screenshot.
mCollector.setTimedOut(true);
mCollector.maybePostResult();
UiUtils.settleDownUI(InstrumentationRegistry.getInstrumentation());
// Wait until the callback has been called.
// The result from the connectivity task should trigger the callback since we shouldn't be
// waiting for the screenshot.
Assert.assertTrue(
"Failed to acquire semaphore.", semaphore.tryAcquire(5, TimeUnit.SECONDS));
Assert.assertTrue("Result should be ready after retrieving all data.", hasResult.get());
Expand Down

0 comments on commit ac57656

Please sign in to comment.