Passing tests sometimes detected as failing |
|||||||||
Issue descriptionI've noticed this with a few different tests, but this one is the only example I have handy: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_ChromiumOS_MSan_Tests%2F105%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FAutomationApiTest.DesktopHitTest%2F0 https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/105 The test says [ OK ] AutomationApiTest.DesktopHitTest (48339 ms) However, at the end of the full test output (https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_ChromiumOS_MSan_Tests%2F105%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Fstdout) it says: [718/718] AutomationApiTest.DesktopHitTest (38847 ms) 1 test produced excessive output: AutomationApiTest.DesktopHitTest (../../chrome/browser/extensions/api/automation/automation_apitest.cc:236) I'm not sure whether that indicates that "excessive output" is causing it to detect a "failure"?
,
Apr 20 2017
Yes, excessive output is considered a failure. See this thread from chromium-dev from last fall: https://groups.google.com/a/chromium.org/d/topic/chromium-dev/ymxI-AaZ7-o/discussion If there's some way we can make this be clearer, I'm certainly open to suggestions.
,
Apr 21 2017
Default P2.
,
Apr 24 2017
If the cause is excessive output, should this still be in the trooper queue?
,
Apr 25 2017
I think this can be removed from the trooper queue. It needs to be triaged as a chromium bug.
,
Apr 27 2017
Coming back to this - perhaps it could be made clearer in the test logs that it's considered a failure, i.e. mark as FAIL rather than OK?
,
Apr 27 2017
Yeah, that might be something we can change. mcgreevy@, am I remembering correctly that you did the log truncation work?
,
Apr 28 2017
Actually, this per-test truncation was done by phajdan, here: https://codereview.chromium.org/2406243004 . I had a related change in progress to limit the total output, but put in on the back-burner to focus on layout test work. The problem here AIUI is that the test itself thinks everything is OK (and generates the "[ OK ]" text) but the test launcher is policing its output length and marking it as failed if the output is excessive. The thing is, we don't want to suppress or modify the actual test output in a case like this (apart from truncating it), since it can be useful in debugging the cause of the excessive output. I think that the correct place to address this is probably in the step output on the builder page, e.g.: browser_tests browser_tests Run on OS: 'Ubuntu-14.04' failures (excessive output): AutomationApiTest.DesktopHitTest rather than the current: browser_tests browser_tests Run on OS: 'Ubuntu-14.04' failures: AutomationApiTest.DesktopHitTest If others agree with that, I can take a look at it on Monday.
,
May 4 2017
That seems like a reasonable solution.
,
May 12 2017
mcgreevy@, are you owning this?
,
May 16 2017
@rdevlin: Yes. I hope to take a look at it later this week.
,
Jun 6 2017
,
May 4 2018
Unassigning myself, since I won't be working on this.
,
May 4 2018
,
May 8 2018
I'm not sure it's in the realm of Infra>Client>Chrome - +jbudorick for an opinion. Other than that, I'm lowering the priority (since this bug had very low activity in a while), and putting it into our backlog hotlist. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by iannu...@google.com
, Apr 20 2017