Bug 147520

Summary: Improve some LayoutTests/http/tests/appcache tests, reduce flakey output
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: ap, commit-queue, joepeck, mkwst
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=153503
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[WIP] Attempted to Reset Server and Redirect to Test none

Description Joseph Pecoraro 2015-07-31 19:55:26 PDT 
* SUMMARY
A few LayoutTests/http/tests/appcache tests are flakey when they include Console output:

  LayoutTests/http/tests/appcache/fail-on-update.html
  LayoutTests/http/tests/appcache/fail-on-update-2.html

Depending on the state of the server the test might fetch 1-2 404 Manifest resources, resulting in 1-2 Console Messages. We can have the test suppress console message output, as it isn't important for the test.

Note, other appcache tests are not flakey in this way.
Comment 1 Joseph Pecoraro 2015-07-31 19:58:29 PDT 
Created attachment 257990 [details]
[PATCH] Proposed Fix
Comment 2 Alexey Proskuryakov 2015-08-01 14:55:48 PDT 
Could you please tell more about it the state that can make the results different? Can it be reset between tests?

Note, I didn't look at these tests now, so maybe it's an obvious silly question - sorry if that's the case.
Comment 3 Joseph Pecoraro 2015-08-03 14:20:39 PDT 
(In reply to comment #2)
> Could you please tell more about it the state that can make the results
> different? Can it be reset between tests?

The tests have a static manifest URL like:

    <html manifest="resources/fail-on-update.php">

It looks like ultimately they test the behavior of applicationCache.update(), but only if the server's response for that manifest URL was what they needed. When the test runs, if the server's manifest response is not what it needed it sends a sync XHR to the server to change its state, and reloads the page until it gets the manifest response's state it wants.

Because the state is on the server, and the URL is a static URL that doesn't change, then running this test in parallel can be problematic. If two workers run the same test they may both be modifying the state of the server, or on repeated loads the initial state of the server may be different than normal.

Since I'm unsure what ASSERT conditions were being tested by the test, I didn't investigate rewriting the test in a different way. It seemed the only flakiness was the new CONSOLE MESSAGE output, so disabling that seemed reasonable.
Comment 4 Alexey Proskuryakov 2015-08-03 15:20:54 PDT 
Ugh, we should not have multiple tests use the same state server side! Did I overlook these tests?
Comment 5 Alexey Proskuryakov 2015-08-03 15:22:34 PDT 
fail-on-update-2 uses a different manifest, resources/fail-on-update-2.php.
Comment 6 Joseph Pecoraro 2015-08-03 20:14:15 PDT 
(In reply to comment #5)
> fail-on-update-2 uses a different manifest, resources/fail-on-update-2.php.

So, I tried adding back the original ASSERT, and was unable to trigger it with these tests. We could tweak the test, but I'm not sure what it was originally testing is actually still being tested anymore.
Comment 7 Alexey Proskuryakov 2015-08-03 21:33:22 PDT 
I think that part of the answer may be to have a non-cached main resource that resets the state, and then navigates to the actual test.
Comment 8 Joseph Pecoraro 2015-08-04 16:25:27 PDT 
(In reply to comment #7)
> I think that part of the answer may be to have a non-cached main resource
> that resets the state, and then navigates to the actual test.

I just tried an approach that resets the server, then navigates and runs the real test. It was still flakey locally (1 out of 200 failures). I'm going to just mark this test as flakey. I'll attach my WIP patch.
Comment 9 Joseph Pecoraro 2015-08-04 16:26:31 PDT 
Created attachment 258230 [details]
[WIP] Attempted to Reset Server and Redirect to Test