Bug 46714 – WebKitTestRunner needs layoutTestController.setCallCloseOnWebViews

• Component: Tools / Tests
• Status: NEW

[Adam Roben (:aroben)]

WebKitTestRunner needs layoutTestController.setCallCloseOnWebViews

[Adam Roben (:aroben)]

<rdar://problem/8485908>

[Balazs Kelemen]

Currently it is on the win-wk2 list and also on the mac-snowleopard list with other reason. It should be skipped via the mac-wk2 list to make it skipped on every wk2 platform.

[Balazs Kelemen]

Created attachment 98714 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 98714 [details]
Patch

r=me. Please reopen the bug after CQ closed it. :)

[WebKit Review Bot]

Comment on attachment 98714 [details]
Patch

Rejecting attachment 98714 [details] from commit-queue.

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

Last 500 characters of output:
cripts/webkitpy/common/system/executive.py", line 385, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 441, 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/8944341

[Andras Becsi]

(In reply to comment #5)
> (From update of attachment 98714 [details])
> Rejecting attachment 98714 [details] from commit-queue.
> 
> Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1
> 
> Last 500 characters of output:
> cripts/webkitpy/common/system/executive.py", line 385, in run_command
>     close_fds=self._should_close_fds())
>   File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 441, 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/8944341

Seems, that CQ does not like UTF-8 characters in reviewers names, like in bug
https://bugs.webkit.org/show_bug.cgi?id=61890

[Balazs Kelemen]

Created attachment 98719 [details]
Patch

One more test is affected by this and need to be skipped.

[Eric Seidel (no email)]

(In reply to comment #6)
> Seems, that CQ does not like UTF-8 characters in reviewers names, like in bug
> https://bugs.webkit.org/show_bug.cgi?id=61890

I suspect it's a config issue on these new CQ bots, due to setting LANG=ascii, etc.

[Csaba Osztrogonác]

Comment on attachment 98719 [details]
Patch

rs=me, but it seems your patch needs update to be appliable.

[Balazs Kelemen]

Committed r89900: <http://trac.webkit.org/changeset/89900>

[Andras Becsi]

(In reply to comment #10)
> Committed r89900: <http://trac.webkit.org/changeset/89900>

Can this bug be closed now?

[Andras Becsi]

(In reply to comment #11)
> (In reply to comment #10)
> > Committed r89900: <http://trac.webkit.org/changeset/89900>
> 
> Can this bug be closed now?

Please disregard me, this was just skipping the affected tests.
Please make it more obvious next time, that the patch only skips tests.

[Hugo Parente Lima]

Is this function really needed?

Only two layouttests uses it:

./plugins/open-and-close-window-with-plugin.html
./plugins/geturlnotify-during-document-teardown.html


The chromium implementation does nothing:

void LayoutTestController::setCallCloseOnWebViews(const CppArgumentList& arguments, CppVariant* result)
{
    result->setNull();
}

The Qt implementation also does nothing.

DumpRenderTree/LayoutTestController.{h|cpp} just sets the variable m_callCloseOnWebViews that isn't used anywhere.

The other ports didn't have any reference to it.

So it's fine to remove it and modify the only two layout tests using it?

Regards

[Balazs Kelemen]

> So it's fine to remove it and modify the only two layout tests using it?
 
I think's it's definitely an improvement to remove such a non-sense feature.

[Hugo Parente Lima]

(In reply to comment #14)
> > So it's fine to remove it and modify the only two layout tests using it?
> 
> I think's it's definitely an improvement to remove such a non-sense feature.

I was wrong, some ports including Qt-WK1 really do something on this function, so it's still valid.

Sorry for the noise.