Bug 36531 – [Qt][GTK] http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zero-length.html fails after r56394

• Status: CLOSED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

http/tests/xmlhttprequest/xmlhttprequest-onProgress-open-should-zero-length.html 
 skipped by http://trac.webkit.org/changeset/56441 (Qt and GTK)

[Julien Chaffraix]

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.

[Julien Chaffraix]

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).

[Csaba Osztrogonác]

(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)

[Gustavo Noronha (kov)]

(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!

[Julien Chaffraix]

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).

[Eric Seidel (no email)]

This looks like something Alexey would be a good reviewer for.

[Julien Chaffraix]

Created attachment 51635 [details]
Proposed fix - 2: same as previously but with the updated Skipped lists

[WebKit Commit Bot]

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 [e-mail]...
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).

[Alexey Proskuryakov]

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.

[Julien Chaffraix]

Landed the fix in r56550.

[Julien Chaffraix]

Just to make it clear, I amended the test to address Alexey's comment.

[Simon Hausmann]

cherry-pick-for-backport: <r56550>

[Simon Hausmann]

Revision r56550 cherry-picked into qtwebkit-2.0 with commit f05e1c8b16e4293df56aca0af2f07089c95ee735

[Robert Hogan]

Created attachment 73525 [details]
Patch

[Robert Hogan]

Comment on attachment 73525 [details]
Patch

Sorry - wrong bug.