Bug 63057 - [Chromium] perf/array-binary-search.html fails as MISSING
: [Chromium] perf/array-binary-search.html fails as MISSING
Status: RESOLVED WONTFIX
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-21 01:51 PDT by Yuta Kitamura
Modified: 2013-04-09 16:28 PDT (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2011-06-21 01:51:35 PDT
Recently, perf/array-binary-search.html started to frequently fail as MISSING:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=perf%2Farray-binary-search.html&group=%40ToT%20-%20chromium.org

I find r89302 (bug 62753) is suspicious. Dirk, could you take a look at this?
Comment 1 Ojan Vafai 2011-06-21 10:38:27 PDT
It looks like it's getting a JS error:

+CONSOLE MESSAGE: line 46: Uncaught ReferenceError: Magnitude is not defined

Looking at resources/magnitude-perf.js, this must mean the JS file is not getting loaded when this code is hit. Looks to me like we likely have a bug where we're not waiting for the script to load before executing the next script.

Seems likely that it was introduced around http://trac.webkit.org/log/?verbose=on&rev=89312&stop_rev=89303. Might be http://trac.webkit.org/changeset/89312/ ?
Comment 2 Dirk Pranke 2011-06-21 12:46:27 PDT
it seems odd that that would be reporting a MISSING result, though.
Comment 3 Dirk Pranke 2011-06-21 14:48:11 PDT
looks like the MISSING failures are flaky, and this could happen in this case if the script load is failing before dumpAsText() is getting called ... we generate text, but don't know that no image file is expected, and thus complain that there's no image expectation.

I feel like in the past we used to treat this as an IMAGE failure, not a MISSING failure (MISSING was reserved for only missing text), but maybe I'm wrong. 

Ojan/Tony, do you recall one way or another?
Comment 4 Tony Chang 2011-06-21 15:16:42 PDT
I don't recall ever making MISSING specific to only text.  Seems like we should just fix the test to not be flaky.
Comment 5 Dirk Pranke 2011-06-21 15:28:24 PDT
Okay, looking back in the repository, it does look like MISSING could've been either from missing text, image, or image hash. However, in the old version of the output, we could use the test_failures to actually print which kind of failure it was. In the new output, we're missing that.

IIRC, this was a conscious decision in that we thought that this didn't happen very often. However, I feel like I've been seeing this pretty often lately, so maybe this is worth revisiting/changing.
Comment 6 Tony Chang 2011-06-21 15:42:14 PDT
FWIW, I think it's better to just fix all the MISSING errors as they come up.  They're normally trivial to fix and making it easy to suppress the errors in test_expectations.txt seems to cause the errors to go unnoticed.

If you need help generating a result for a platform, you can just ask the person who added the test (I've had success with this in the past).
Comment 7 Ojan Vafai 2011-06-21 15:51:38 PDT
In this case, it looks like an actual WebCore bug. The fact that it shows up in this weird way in the test runner is coincidental and I don't think it's worth fixing.

abarth, it seems like this might possibly be fallout from your patch. You have any ideas?
Comment 8 Dirk Pranke 2011-06-21 16:01:49 PDT
I feel like maybe I'm being unclear here ...

This test isn't missing any expected results at all. The fact that it's being reported as missing is misleading. It should be being reported as a TEXT failure, but it's being reported as a MISSING failure because for some reason dumpAsText() didn't get called before the test exited due to an exception being raised. So, the fact that it's showing up this way in the layout-test-results isn't coincidental at all.

But, as a result of the changes in the results.html formatting, the HTML is confusing and it will send people off on a wild goose chase trying to figure out why their expected results are "missing".

This all seems bad to me, and I feel like I'm seeing this on a surprising number of results pages and test runs lately (i.e., this is not a particularly unusual situation).

Does that all make sense?
Comment 9 Adam Barth 2011-06-21 16:02:54 PDT
> abarth, it seems like this might possibly be fallout from your patch. You have any ideas?

Really?  Could be.  The FrameLoader is very hairy code, but I don't see the connection.  Does the problem go away when the patch is reverted locally?
Comment 10 Ojan Vafai 2011-06-21 16:07:20 PDT
(In reply to comment #9)
> > abarth, it seems like this might possibly be fallout from your patch. You have any ideas?
> 
> Really?  Could be.  The FrameLoader is very hairy code, but I don't see the connection.  Does the problem go away when the patch is reverted locally?

I was just looking at when this got flaky in the dashboard. All the signs point to that patch. But, in either case, this points to a bug, right? I just CC'ed you, eseidel and tonyg as the people who might have insight into what changed recently to cause the bug. It's essentially the following case:

<script src="foo.js"></script>
<script>
// Use something from foo.js.
</script>

We decide the test is complete (i.e. after the onload finishes) and run the second script before foo.js has loaded. Is that not a bug?
Comment 11 Adam Barth 2011-06-21 16:15:23 PDT
> We decide the test is complete (i.e. after the onload finishes) and run the second script before foo.js has loaded. Is that not a bug?

Yes.  That's a very serious bug.  The only thing I can think of is that the first load fails somehow and then we continue on.
Comment 12 Ojan Vafai 2011-06-21 16:23:27 PDT
(In reply to comment #11)
> > We decide the test is complete (i.e. after the onload finishes) and run the second script before foo.js has loaded. Is that not a bug?
> 
> Yes.  That's a very serious bug.  The only thing I can think of is that the first load fails somehow and then we continue on.

This got flaky on all the chromium bots around the same revision. The only overlap I saw at a quick glance was your FrameLoader patch. Of course, since it's flaky, it could have been earlier.
Comment 13 Dirk Pranke 2011-06-21 16:24:37 PDT
I have filed a separate bug (bug 63104) to discuss the way these failures are being handled by DRT and NRWT.
Comment 14 Tony Gentilcore 2011-06-26 04:30:22 PDT
> <script src="foo.js"></script>
> <script>
> // Use something from foo.js.
> </script>
> 
> We decide the test is complete (i.e. after the onload finishes) and run the second script before foo.js has loaded. Is that not a bug?

How reproducible is this locally? Can you see if onerror is firing for the foo.js script?
Comment 15 Steve Block 2011-11-17 04:50:48 PST
This failure mode is occurring again, so I'm adding a MISSING expectation for Chromium.
Comment 16 Steve Block 2011-11-17 05:01:38 PST
Committed r100606: <http://trac.webkit.org/changeset/100606>
Comment 17 Kent Tamura 2012-01-24 04:53:21 PST
*** Bug 76900 has been marked as a duplicate of this bug. ***
Comment 18 Stephen Chenney 2013-04-09 16:28:00 PDT
Marking test failures as WontFix. Bug is still accessible and recording in TestExpectations.