Bug 36531 - [Qt][GTK] http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zero-length.html fails after r56394
Summary: [Qt][GTK] http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zer...
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt
Depends on:
Blocks: 18654 35784
  Show dependency treegraph
 
Reported: 2010-03-24 06:51 PDT by Csaba Osztrogonác
Modified: 2010-11-10 12:39 PST (History)
6 users (show)

See Also:


Attachments
Proposed fix: remove bogus assumption in the test (2.34 KB, patch)
2010-03-24 09:34 PDT, Julien Chaffraix
jchaffraix: commit-queue-
Details | Formatted Diff | Diff
Proposed fix - 2: same as previously but with the updated Skipped lists (3.76 KB, patch)
2010-03-25 07:07 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Patch (4.22 KB, patch)
2010-11-10 12:34 PST, Robert Hogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2010-03-24 06:51:17 PDT
http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zero-length.html 
fails consistently on Qt and on GTK after r56394.

original bug: https://bugs.webkit.org/show_bug.cgi?id=18654
(Check it, there are some important comment here about this bug.)

It might be a regression or a hidden DRT bug revealed by this change.
Comment 1 Csaba Osztrogonác 2010-03-24 07:12:16 PDT
http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zero-length.html 
 skipped by http://trac.webkit.org/changeset/56441 (Qt and GTK)
Comment 2 Julien Chaffraix 2010-03-24 09:24:15 PDT
In both cases (GTK and Qt), the test times out.

Looking at the failed test, it is expecting to get at least 3 progress events which may not be the case anymore due to bug 18654 (we throttle the progress events to one per 50 ms or for every bits the slower of the two previous). So I think this is a test issue.
Comment 3 Julien Chaffraix 2010-03-24 09:34:33 PDT
Created attachment 51514 [details]
Proposed fix: remove bogus assumption in the test

Tested this on my mac without any problem.

I could not reproduce the issue on my linux box so it is prospective (but based on Ossy's testing of the idea behind this patch).
Comment 4 Csaba Osztrogonác 2010-03-24 09:44:59 PDT
(In reply to comment #3)
> Created an attachment (id=51514) [details]
> Proposed fix: remove bogus assumption in the test
Thanks for fix, it works for me. (Qt-linux)
Comment 5 Gustavo Noronha (kov) 2010-03-24 11:05:05 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=51514) [details] [details]
> > Proposed fix: remove bogus assumption in the test
> Thanks for fix, it works for me. (Qt-linux)

Works in my GTK+ build as well, thanks!
Comment 6 Julien Chaffraix 2010-03-24 12:26:05 PDT
Comment on attachment 51514 [details]
Proposed fix: remove bogus assumption in the test

Great, this patch fixes both builds :)

Thanks for the testing guys!

Removing the commit-queue flag as it is missing the Skipped files update (I will add it before landing).
Comment 7 Eric Seidel (no email) 2010-03-25 00:01:08 PDT
This looks like something Alexey would be a good reviewer for.
Comment 8 Julien Chaffraix 2010-03-25 07:07:43 PDT
Created attachment 51635 [details]
Proposed fix - 2: same as previously but with the updated Skipped lists
Comment 9 WebKit Commit Bot 2010-03-25 07:20:55 PDT
Comment on attachment 51635 [details]
Proposed fix - 2: same as previously but with the updated Skipped lists

Rejecting patch 51635 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', '--build-style=both', '--quiet', '51635', '--no-update']" exit_code: 1
Logging in as eseidel@chromium.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=51635&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=36531&ctype=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Processing patch 51635 from bug 36531.
ERROR: LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" (case insensitive).
Comment 10 Alexey Proskuryakov 2010-03-25 09:01:10 PDT
Comment on attachment 51635 [details]
Proposed fix - 2: same as previously but with the updated Skipped lists

> +        log((lastPosition == resourceSize) ? "PASSED" : ("FAILED: expected 1461754, actual " + lastPosition));

Please replace the hardcoded "1461754" string with resourceSize.
Comment 11 Julien Chaffraix 2010-03-25 09:34:00 PDT
Landed the fix in r56550.
Comment 12 Julien Chaffraix 2010-03-25 09:34:59 PDT
Just to make it clear, I amended the test to address Alexey's comment.
Comment 13 Simon Hausmann 2010-03-26 06:50:26 PDT
cherry-pick-for-backport: <r56550>
Comment 14 Simon Hausmann 2010-03-26 06:53:53 PDT
Revision r56550 cherry-picked into qtwebkit-2.0 with commit f05e1c8b16e4293df56aca0af2f07089c95ee735
Comment 15 Robert Hogan 2010-11-10 12:34:45 PST
Created attachment 73525 [details]
Patch
Comment 16 Robert Hogan 2010-11-10 12:39:57 PST
Comment on attachment 73525 [details]
Patch

Sorry - wrong bug.