Bug 36934

Summary:

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

Product:

WebKit

Reporter:

Eric Seidel (no email) <eric>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ap, commit-queue, jchaffraix

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.4

Bug Depends on:

18654

Bug Blocks:

Attachments:

Description	Flags
Add more explicit comments to the test for debugging this issue	none
Patch	none

Eric Seidel (no email)

Reported 2010-03-31 23:53:33 PDT

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)

Attachments
Add more explicit comments to the test for debugging this issue (3.16 KB, patch) 2010-04-01 08:41 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Patch (1.32 KB, patch) 2010-04-02 23:03 PDT, Adam Barth	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2010-03-31 23:54:30 PDT

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

Comment 2 2010-04-01 08:37:36 PDT

(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

Comment 3 2010-04-01 08:41:47 PDT

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

Eric Seidel (no email)

Comment 4 2010-04-01 11:22:41 PDT

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

Comment 5 2010-04-01 11:49:51 PDT

(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 6 2010-04-01 11:57:26 PDT

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

WebKit Commit Bot

Comment 7 2010-04-01 12:23:45 PDT

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

Comment 8 2010-04-01 12:23:52 PDT

All reviewed patches have been landed. Closing bug.

Julien Chaffraix

Comment 9 2010-04-01 20:15:38 PDT

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)

Comment 10 2010-04-01 20:20:46 PDT

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

Comment 11 2010-04-01 20:44:17 PDT

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

Eric Seidel (no email)

Comment 12 2010-04-02 00:36:44 PDT

(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

Comment 13 2010-04-02 08:27:17 PDT

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)

Comment 14 2010-04-02 09:58:14 PDT

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

Comment 15 2010-04-02 15:17:22 PDT

(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

Comment 16 2010-04-02 15:20:36 PDT

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

Comment 17 2010-04-02 23:03:50 PDT

Created attachment 52483 [details] Patch

Adam Barth

Comment 18 2010-04-02 23:15:12 PDT

Comment on attachment 52483 [details] Patch Clearing flags on attachment: 52483 Committed r57044: <http://trac.webkit.org/changeset/57044>

Adam Barth

Comment 19 2010-04-02 23:34:51 PDT

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

Alexey Proskuryakov

Comment 20 2010-04-05 10:26:02 PDT

Great! Can this bug be marked as resolved now?

Eric Seidel (no email)

Comment 21 2010-04-05 10:30:36 PDT

Looks resolved to me.

Note You need to log in before you can comment on or make changes to this bug.