Bug 63255 - [Qt]REGRESSION(r90471): It made 4 fast/notifications tests fail on Qt
Summary: [Qt]REGRESSION(r90471): It made 4 fast/notifications tests fail on Qt
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Critical
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 62066
  Show dependency treegraph
 
Reported: 2011-06-23 07:40 PDT by Csaba Osztrogonác
Modified: 2011-07-11 06:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (3.44 KB, patch)
2011-07-08 12:04 PDT, Yael
ossy: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Csaba Osztrogonác 2011-06-23 07:47:10 PDT
I skipped the failing tests: http://trac.webkit.org/changeset/89569

But it doesn't solve the real bug which should be fixed or the original patch should be rolled out.
Comment 2 Nate Chapin 2011-06-23 10:20:44 PDT
r89503 changed the timings of the callback that tells the embedder that a load is complete.  From the layout test diffs you linked, it looks like the notifications aren't being completed before the test finishes and the next one starts.

Feel free to grab me on IRC if there's anything I can do to help with this.
Comment 3 Yael 2011-06-23 19:11:25 PDT
Ossy, since r89503 was mostly reverted, should we unskip the notifications tests?
Comment 4 Csaba Osztrogonác 2011-06-27 01:54:45 PDT
(In reply to comment #3)
> Ossy, since r89503 was mostly reverted, should we unskip the notifications tests?

They were unskipped (accidentally :)) ): http://trac.webkit.org/changeset/89732/trunk/LayoutTests/platform/qt/Skipped
Comment 5 Andras Becsi 2011-07-07 10:17:41 PDT
The tests started to fail again after http://trac.webkit.org/changeset/90471:

fast/notifications/notifications-no-icon.html (flaky)
fast/notifications/notifications-double-show.html
fast/notifications/notifications-with-permission.html
fast/notifications/notifications-without-permission.html

Skipped tests in http://trac.webkit.org/changeset/90570 and reopening the bug.

http://build.webkit.org/results/Qt%20Linux%20Release/r90560%20(35105)/results.html
Comment 6 Yael 2011-07-08 10:46:28 PDT
Before http://trac.webkit.org/changeset/89569, we would wait for the notification icon to load before transitioning to committed state, and the test had enough time to complete. With http://trac.webkit.org/changeset/89569, we transition to committed state before the load is finished, thus the tests are failing.

I think the behavior of Notifications is still correct, but the tests need to be modified, so that they wait for the icon to finish loading.

I am preparing a patch for that.
Comment 7 Yael 2011-07-08 12:04:31 PDT
Created attachment 100144 [details]
Patch

Modified the tests to wait until icon loading is finished.
Comment 8 Csaba Osztrogonác 2011-07-11 06:19:14 PDT
Comment on attachment 100144 [details]
Patch

LGTM, r=me
Comment 9 WebKit Review Bot 2011-07-11 06:20:46 PDT
Comment on attachment 100144 [details]
Patch

Rejecting attachment 100144 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 423, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 479, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/9013318
Comment 10 Csaba Osztrogonác 2011-07-11 06:24:00 PDT
(In reply to comment #9)
> (From update of attachment 100144 [details])
> Rejecting attachment 100144 [details] from commit-queue.
> 
> Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 1
> 
> Last 500 characters of output:
> cripts/webkitpy/common/system/executive.py", line 423, in run_command
>     close_fds=self._should_close_fds())
>   File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 479, in popen
>     return subprocess.Popen(*args, **kwargs)
>   File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
>     errread, errwrite)
>   File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
>     raise child_exception
> TypeError: execv() arg 2 must contain only strings
> 
> Full output: http://queues.webkit.org/results/9013318

CQ still hates my name. :(( Could you land it manually?
Comment 11 WebKit Review Bot 2011-07-11 06:31:52 PDT
Comment on attachment 100144 [details]
Patch

Rejecting attachment 100144 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 1

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 423, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 479, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

Full output: http://queues.webkit.org/results/9014194
Comment 12 Yael 2011-07-11 06:59:07 PDT
Committed r90471: <http://trac.webkit.org/changeset/90741>