WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
133780
[GTK] Performance tests should be always ran with WKTR
https://bugs.webkit.org/show_bug.cgi?id=133780
Summary
[GTK] Performance tests should be always ran with WKTR
Carlos Alberto Lopez Perez
Reported
2014-06-11 18:12:45 PDT
The GTK port only supports WebKit2. Therefore the --webkit-test-runner/-2 switch for Tools/Scripts/run-perf-tests should be always enabled on this platform.
Attachments
Patch
(1.63 KB, patch)
2014-06-11 18:18 PDT
,
Carlos Alberto Lopez Perez
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Carlos Alberto Lopez Perez
Comment 1
2014-06-11 18:18:51 PDT
Created
attachment 232926
[details]
Patch
Ryosuke Niwa
Comment 2
2014-06-11 22:13:43 PDT
Comment on
attachment 232926
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=232926&action=review
> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:71 > + # The GTK+ port only supports WebKit2, so it always uses WKTR. > + if self._port.name().startswith("gtk"): > + self._options.webkit_test_runner = True
Should we split out some warning about this?
Manuel Rego Casasnovas
Comment 3
2014-06-12 00:18:44 PDT
(In reply to
comment #2
)
> (From update of
attachment 232926
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=232926&action=review
> > > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:71 > > + # The GTK+ port only supports WebKit2, so it always uses WKTR. > > + if self._port.name().startswith("gtk"): > > + self._options.webkit_test_runner = True > > Should we split out some warning about this?
For example right now "run-launcher --gtk -2" fails and returns: Starting webkit launcher. Cannot parse arguments: Unknown option -2 I don't know what's the best approach, maybe simply showing a warning and using always WebKit2 is the best option. But I think we should be consistent and do the same in: run-launcher, run-webkit-tests and run-perf-tests. Of course, this can be done in different patches.
Carlos Alberto Lopez Perez
Comment 4
2014-06-12 04:47:07 PDT
(In reply to
comment #3
)
> (In reply to
comment #2
) > > (From update of
attachment 232926
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=232926&action=review
> > > > > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:71 > > > + # The GTK+ port only supports WebKit2, so it always uses WKTR. > > > + if self._port.name().startswith("gtk"): > > > + self._options.webkit_test_runner = True > > > > Should we split out some warning about this? > > For example right now "run-launcher --gtk -2" fails and returns: > Starting webkit launcher. > Cannot parse arguments: Unknown option -2 > > I don't know what's the best approach, maybe simply showing a warning and using always WebKit2 is the best option. > But I think we should be consistent and do the same in: run-launcher, run-webkit-tests and run-perf-tests. Of course, this can be done in different patches.
The bots are going to keep passing "--webkit-test-runner/-2" because that is how are implemented the steps on Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg Making it fail when "-2/--webkit-test-runner" is passed will break the bots. And I'm not sure about the usefulness of showing a warning. run-webkit-tests also uses WKTR always with GTK+ and no warning is shown: <
http://trac.webkit.org/changeset/167426
>. So I think the current patch is consistent with that.
Manuel Rego Casasnovas
Comment 5
2014-06-12 05:23:30 PDT
Comment on
attachment 232926
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=232926&action=review
>>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:71 >>>> + self._options.webkit_test_runner = True >>> >>> Should we split out some warning about this? >> >> For example right now "run-launcher --gtk -2" fails and returns: >> Starting webkit launcher. >> Cannot parse arguments: Unknown option -2 >> >> I don't know what's the best approach, maybe simply showing a warning and using always WebKit2 is the best option. >> But I think we should be consistent and do the same in: run-launcher, run-webkit-tests and run-perf-tests. Of course, this can be done in different patches. > > The bots are going to keep passing "--webkit-test-runner/-2" because that is how are implemented the steps on Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg Making it fail when "-2/--webkit-test-runner" is passed will break the bots. > > And I'm not sure about the usefulness of showing a warning. run-webkit-tests also uses WKTR always with GTK+ and no warning is shown: <
http://trac.webkit.org/changeset/167426
>. So I think the current patch is consistent with that.
Ok, fair enough. I didn't realize run-webkit-tests was fixed already. Maybe we could change run-launcher to use WebKit2 by default and not fail (maybe showing a warning in that particular case).
WebKit Commit Bot
Comment 6
2014-06-12 05:53:54 PDT
Comment on
attachment 232926
[details]
Patch Clearing flags on attachment: 232926 Committed
r169893
: <
http://trac.webkit.org/changeset/169893
>
WebKit Commit Bot
Comment 7
2014-06-12 05:53:59 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug