Bug 36934 – REGRESSION(56394): http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html fails on Tiger

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

REGRESSION(56394): http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html fails on Tiger

This test has been failing since it was committed.  It may just need Tiger-specific results checked in, but that is very unclear from the (not very helpful) pass/fail output.

http://build.webkit.org/results/Tiger%20Intel%20Release/r56893%20(10281)/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-diffs.txt

--- /Volumes/Data/WebKit-BuildSlave/tiger-intel-release/build/layout-test-results/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-expected.txt	2010-03-31 21:39:46.000000000 -0700
+++ /Volumes/Data/WebKit-BuildSlave/tiger-intel-release/build/layout-test-results/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-actual.txt	2010-03-31 21:39:46.000000000 -0700
@@ -5,8 +5,8 @@
 You should see a sequence of 5 PASSED.
 
 PASSED
+FAILED (count was 1)
 PASSED
 PASSED
-PASSED
-PASSED
+FAILED (count was 1)

[Eric Seidel (no email)]

I see, the diff is just hard to read.  Tiger is simply failing 2 of the 5 subtests.  My "not very helpful" comment above was itself not very helpful.  What I meant was that the PASSED/FAILED messages were not self-documenting.

[Julien Chaffraix]

(In reply to comment #1)
> I see, the diff is just hard to read.  Tiger is simply failing 2 of the 5
> subtests.  My "not very helpful" comment above was itself not very helpful. 
> What I meant was that the PASSED/FAILED messages were not self-documenting.

You are right about the message. I will attach a patch to have more explicit messages that will help find out what is failing. It is likely the 2 tests that fails are the 2 throttled ones, the other ones just make sure there is no throttling.

Thanks for filing this bug. I will be more careful with the Tiger build-bot next time.

[Julien Chaffraix]

Created attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

[Eric Seidel (no email)]

Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

We would need to update the expected results when checking this in if we didn't want failures.

[Julien Chaffraix]

(In reply to comment #4)
> (From update of attachment 52299 [details])
> We would need to update the expected results when checking this in if we didn't
> want failures.

No, I only made the failures more clear.
The test does asynchronous XHRs which means that saying which one passed is not reliable 100% of the time. There is value in keeping it as such (like checking that the throttling will still work properly with several concurrent XHRs). That's why I did not want to update the expected results.

[Eric Seidel (no email)]

Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

OK.

[WebKit Commit Bot]

Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

Clearing flags on attachment: 52299

Committed r56928: <http://trac.webkit.org/changeset/56928>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Julien Chaffraix]

Reopening this bug as it was not mean to be closed.

The Tiger build bot fails consistently 2 strict tests (my previous analysis was completely off):

FAILED (count was 1) for strict test 1
FAILED (count was 1) for strict test 3

Looking at http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html, the only strict test that passes is expecting 1 'progress' event. The throttled tests would also pass with only 1 event. Without more information, it seems like the network library buffers the input.

Increasing the time between chunks of data could help but we may miss some corner cases at the 50 ms limit (which would be difficult to get reliably on the bots). I will go ahead and increase it in the following days to make the bot happy.

[Eric Seidel (no email)]

I do seem to remember Tiger's CFNetwork having some sort of buffer for loading http requests off the network.  But I could be remembering wrong.  AP or some other Apple person would know.

[Alexey Proskuryakov]

Yes, our code that disables content sniffing (and thus buffering) only works on Leopard and later. Good catch!

[Eric Seidel (no email)]

(In reply to comment #11)
> Yes, our code that disables content sniffing (and thus buffering) only works on
> Leopard and later. Good catch!

So I'm unclear from your remark.  Is there a code fix to make here, or just a test fix?

[Alexey Proskuryakov]

I was answering your question in comment 10. Certainly XMLHttpRequest behavior is different on Tiger because of buffering, but I haven't investigated if test failures are caused by this difference.

[Eric Seidel (no email)]

Adam Barth fixed another failing test in Tiger last night by moving it from using GET to POST. http://trac.webkit.org/changeset/56979   I'm not sure if the same workaround is useful in this context or not.

[Julien Chaffraix]

(In reply to comment #14)
> Adam Barth fixed another failing test in Tiger last night by moving it from
> using GET to POST. http://trac.webkit.org/changeset/56979   I'm not sure if the
> same workaround is useful in this context or not.

I think this would apply here too. I tried such a change and it did not make the test fail. I will create a patch tonight (I cannot come back to that before).

[Alexey Proskuryakov]

Oh, I think that we could perhaps use a non-text MIME type to disable buffering. My recollection is that CFNetwork only does content sniffing for types like text/plain, because that often stands for "unknown".

[Adam Barth]

Created attachment 52483 [details]
Patch

[Adam Barth]

Comment on attachment 52483 [details]
Patch

Clearing flags on attachment: 52483

Committed r57044: <http://trac.webkit.org/changeset/57044>

[Adam Barth]

ap for the win.  Seems to have worked.  Welcome to a green Tiger bot.

[Alexey Proskuryakov]

Great! Can this bug be marked as resolved now?

[Eric Seidel (no email)]

Looks resolved to me.