Bug 142325 - run-webkit-tests and run-perf-tests should use WebKitTestRunner by default
Summary: run-webkit-tests and run-perf-tests should use WebKitTestRunner by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-04 21:24 PST by Chris Dumez
Modified: 2015-03-06 09:09 PST (History)
6 users (show)

See Also:


Attachments
Patch (21.62 KB, patch)
2015-03-04 21:33 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (30.01 KB, patch)
2015-03-05 10:48 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-03-04 21:24:38 PST
run-webkit-tests and run-perf-tests should use WebKitTestRunner by default. As a result, "--webkit-test-runner / -2" parameters would be replaced by "--dump-render-tree / -1" so developers can run DumpRenderTree rather than WebKitTestRunner.
Comment 1 Chris Dumez 2015-03-04 21:33:48 PST
Created attachment 247929 [details]
Patch
Comment 2 Alexey Proskuryakov 2015-03-05 00:29:32 PST
Comment on attachment 247929 [details]
Patch

This patch fails EWS, gotta fix that too.

run_webkit_tests.py: error: no such option: -2
Comment 3 Csaba Osztrogonác 2015-03-05 02:45:30 PST
(In reply to comment #2)
> Comment on attachment 247929 [details]
> Patch
> 
> This patch fails EWS, gotta fix that too.
> 
> run_webkit_tests.py: error: no such option: -2

http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/ports.py#L132

"-2" argument should be removed from MacWK2Port and GtkWK2Port
and --dump-render-tree should be passed to MacPort to run WK1
tests instead of WK2 tests.
Comment 4 Csaba Osztrogonác 2015-03-05 02:56:36 PST
Comment on attachment 247929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247929&action=review

Please update the perf bot names in mastercfg_unittest.py in expected_build_steps variable.

You can run mastercfg_unittest.py by hand in Tools/BuildSlaveSupport/build.webkit.org-config/
after you applied my fix from bug142219, it showed me all issues I commented here.

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:83
> +                    { "name": "Apple Mavericks Debug WK2 (Tests)", "type": "TestAllButJSCFactory", "builddir": "mavericks-debug-tests-wk2",

TestAllButJSCFactory -> TestAllButJSC

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:106
> +                    { "name": "Apple Mavericks Release WK2 (Tests)", "type": "TestAllButJSCFactory", "builddir": "mavericks-release-tests-wk2",

ditto

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:125
> +                    { "name": "Apple Yosemite Debug WK1 (Tests)", "type": "TestWebKit1AllButJSC", "builddir": "yosemite-debug-tests-wk1",

ditto

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:130
> +                    { "name": "Apple Yosemite Debug WK2 (Tests)", "type": "TestAllButJSCFactory", "builddir": "yosemite-debug-tests-wk2",

ditto

> Tools/BuildSlaveSupport/build.webkit.org-config/config.json:157
> +                    { "name": "Apple Yosemite Release WK2 (Tests)", "type": "TestAllButJSCFactory", "builddir": "yosemite-release-tests-wk2",

ditto

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:780
> -    LayoutTestClass = RunWebKitTests
> +    LayoutTestClass = RunWebKit2Tests

There is no RunWebKit2Tests anymore. It should be RunWebKitTests.
Comment 5 Csaba Osztrogonác 2015-03-05 02:57:55 PST
Just out of curiosity, is this change mean that WK1 will be obsolete soon?
Comment 6 Csaba Osztrogonác 2015-03-05 03:08:20 PST
It seems there will be no WK1 perf tester bot after this 
change, you can remove WK1 perf test related crufts too.
Comment 7 Csaba Osztrogonác 2015-03-05 03:14:14 PST
(In reply to comment #4)
> Please update the perf bot names in mastercfg_unittest.py in
> expected_build_steps variable.

It should have been done in http://trac.webkit.org/changeset/181050 .
Comment 8 Chris Dumez 2015-03-05 10:13:43 PST
(In reply to comment #5)
> Just out of curiosity, is this change mean that WK1 will be obsolete soon?

All this means is that WK2 is the focus nowadays and should become the default.

Thanks the reviews, I'll rework the patch.
Comment 9 Chris Dumez 2015-03-05 10:48:22 PST
Created attachment 247968 [details]
Patch
Comment 10 Csaba Osztrogonác 2015-03-05 12:05:11 PST
Comment on attachment 247968 [details]
Patch

LGTM, r=me.
Comment 11 Chris Dumez 2015-03-05 12:11:27 PST
Comment on attachment 247968 [details]
Patch

Clearing flags on attachment: 247968

Committed r181093: <http://trac.webkit.org/changeset/181093>
Comment 12 Chris Dumez 2015-03-05 12:11:31 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Chris Dumez 2015-03-05 13:19:35 PST
The bots on build.webkit.org still try to pass --webkit-test-runner. I don't know why yet :(
Comment 14 Brent Fulgham 2015-03-05 13:22:40 PST
This broke the Windows test builder:

12:06:50.131 224 WebKitTestRunner was not found at /home/buildbot/slave/win-release-tests/build/WebKitBuild/release/bin32/WebKitTestRunner


Windows (unfortunately) does not build WebKitTestRunner.

Can you fix? Or should I roll this out now?
Comment 15 Chris Dumez 2015-03-05 13:24:09 PST
(In reply to comment #14)
> This broke the Windows test builder:
> 
> 12:06:50.131 224 WebKitTestRunner was not found at
> /home/buildbot/slave/win-release-tests/build/WebKitBuild/release/bin32/
> WebKitTestRunner
> 
> 
> Windows (unfortunately) does not build WebKitTestRunner.
> 
> Can you fix? Or should I roll this out now?

This is likely the same issue, some things are out of sync. Win is supposed to pass --dump-render-tree now but apparently doesn't on the bots. And the wk2 bots still pass --webkit-test-runner although they should no longer do this. I am still trying to figure this out.
Comment 16 Chris Dumez 2015-03-05 13:35:25 PST
Ok, the bots should hopefully come back up now.
Comment 17 Csaba Osztrogonác 2015-03-06 09:09:49 PST
It seems the bots should be renamed in perf.webkit.org too.
Ryosuke could you possibly check it?

Uploaded JSON to https://perf.webkit.org/api/report but got an error:
{
    "status": "BuilderNotFound", 
    "failureStored": false, 
    "name": "Apple Yosemite Release WK2 (Perf)", 
    "processedRuns": 0
}

Uploaded JSON to https://perf.webkit.org/api/report but got an error:
{
    "status": "BuilderNotFound", 
    "failureStored": false, 
    "name": "Apple Mavericks Release WK2 (Perf)", 
    "processedRuns": 0
}