WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
77329
[PerformanceTests] Add landing html for Dromaeo dom-query test
https://bugs.webkit.org/show_bug.cgi?id=77329
Summary
[PerformanceTests] Add landing html for Dromaeo dom-query test
Hajime Morrita
Reported
2012-01-30 04:14:25 PST
Adding landing html which include actual test as an iframe.
Attachments
Patch
(7.92 KB, patch)
2012-01-30 04:34 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch
(8.15 KB, patch)
2012-01-30 18:41 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch for landing
(8.29 KB, patch)
2012-01-30 22:48 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch for landing
(8.29 KB, patch)
2012-01-30 22:51 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch for landing
(8.39 KB, patch)
2012-01-31 02:01 PST
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Hajime Morrita
Comment 1
2012-01-30 04:34:40 PST
Created
attachment 124524
[details]
Patch
Hajime Morrita
Comment 2
2012-01-30 04:36:35 PST
Hi Ryosue, could you take a look? I'm intending this to be used for a "proof" test for Dromaeo integration. Once the Dromaeo checkout is visible from run-perf-tests, I'll add other test cases.
Ryosuke Niwa
Comment 3
2012-01-30 09:21:35 PST
Comment on
attachment 124524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124524&action=review
> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 > + re.compile(r'^CONSOLE MESSAGE: line \d+: \d+'),
I'm not sure if we should ignore all console messages since some console messages contain actual errors. What are examples of console messages you get?
> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:255 > + re.compile(r'^Blocked access to external URL'),
What are examples of external URLs Dromaeo tries to load?
> PerformanceTests/ChangeLog:13 > + Reviewed by NOBODY (OOPS!).
This line should appear before the long description.
> PerformanceTests/Dromaeo/dom-query.html:1 > +<html>
Missing DOCTYPE.
> PerformanceTests/Dromaeo/dom-query.html:3 > +<script src="../Checkouts/dromaeo/web/jquery.js"></script>
So Dromaeo will be added to PerformanceTests/Checkouts? Can we instead add it to Dromaeo/resources/? Or license doesn't allow it?
> PerformanceTests/Dromaeo/resources/drtrunner.js:10 > + median += item.median;
This is an approximation at best but I guess it's okay. Or maybe it's better not to report median in this case.
> PerformanceTests/Dromaeo/resources/drtrunner.js:14 > + stdev += item.deviation;
You can't add standard deviations like this. Assuming N_i are mutually independent random variables for all i = 0 through n, stdev of N_0 + N_1 ... + N_n is sqrt(Var[N_0] + Var[N_1] ... + Var[N_n]). So here, you can do: variance += item.deviation * item.deviation;
> PerformanceTests/Dromaeo/resources/drtrunner.js:76 > + DRT.log(""); > + DRT.log("avg " + scores.avg); > + DRT.log("median " + scores.median); > + DRT.log("stdev " + scores.stdev); > + DRT.log("min " + scores.min); > + DRT.log("max " + scores.max);
Maybe we should extract a function from logStatistics so that we don't have to duplicate code here.
> PerformanceTests/Dromaeo/resources/drtrunner.js:80 > + if (window.layoutTestController) > + layoutTestController.notifyDone(); > + }, 0);
Wrong indentation.
Hajime Morrita
Comment 4
2012-01-30 16:29:36 PST
Comment on
attachment 124524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124524&action=review
Hi Ryosuke, thanks for taking a look at this. I'll update this soon. By the way
>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 >> + re.compile(r'^CONSOLE MESSAGE: line \d+: \d+'), > > I'm not sure if we should ignore all console messages since some console messages contain actual errors. What are examples of console messages you get?
I use console.log() to print progress for DRT. It's like "CONSOLE MESSAGE: Line 31: 3091". But I can remove this.
>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:255 >> + re.compile(r'^Blocked access to external URL'), > > What are examples of external URLs Dromaeo tries to load?
This is for icons which test html contains. For example:
https://github.com/omo/dromaeo/blob/master/tests/dom-query.html#L108
>> PerformanceTests/ChangeLog:13 >> + Reviewed by NOBODY (OOPS!). > > This line should appear before the long description.
Oops. will fix.
>> PerformanceTests/Dromaeo/dom-query.html:1 >> +<html> > > Missing DOCTYPE.
Sure.
>> PerformanceTests/Dromaeo/dom-query.html:3 >> +<script src="../Checkouts/dromaeo/web/jquery.js"></script> > > So Dromaeo will be added to PerformanceTests/Checkouts? Can we instead add it to Dromaeo/resources/? Or license doesn't allow it?
Dromaeo specific test cases are MIT license. But I'm thinking we can check out as a part of test execution instead of checking it in statically. If we're going to check it in, resource/ would be better.
>> PerformanceTests/Dromaeo/resources/drtrunner.js:14 >> + stdev += item.deviation; > > You can't add standard deviations like this. Assuming N_i are mutually independent random variables for all i = 0 through n, stdev of N_0 + N_1 ... + N_n is sqrt(Var[N_0] + Var[N_1] ... + Var[N_n]). So here, you can do: > variance += item.deviation * item.deviation;
You are right. Math is hard. Will fix.
>> PerformanceTests/Dromaeo/resources/drtrunner.js:76 >> + DRT.log("max " + scores.max); > > Maybe we should extract a function from logStatistics so that we don't have to duplicate code here.
Will do.
>> PerformanceTests/Dromaeo/resources/drtrunner.js:80 >> + }, 0); > > Wrong indentation.
Will fix.
Hajime Morrita
Comment 5
2012-01-30 16:32:46 PST
> Hi Ryosuke, thanks for taking a look at this. I'll update this soon. > By the way
...By the way, I intended to write something about
Bug 77328
. But I should write there.
Ryosuke Niwa
Comment 6
2012-01-30 16:36:09 PST
Comment on
attachment 124524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124524&action=review
>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 >>> + re.compile(r'^CONSOLE MESSAGE: line \d+: \d+'), >> >> I'm not sure if we should ignore all console messages since some console messages contain actual errors. What are examples of console messages you get? > > I use console.log() to print progress for DRT. It's like "CONSOLE MESSAGE: Line 31: 3091". But I can remove this.
I think we should remove that. There are cases where we want to report errors (e.g. script exceptions) using console message. You can call log(~) as well but I feel that console.log should be a stronger indication of something going wrong.
>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:255 >>> + re.compile(r'^Blocked access to external URL'), >> >> What are examples of external URLs Dromaeo tries to load? > > This is for icons which test html contains. For example:
https://github.com/omo/dromaeo/blob/master/tests/dom-query.html#L108
Huh... can we replace those urls by relative paths if we're checking tests into webkit?
>>> PerformanceTests/Dromaeo/dom-query.html:3 >>> +<script src="../Checkouts/dromaeo/web/jquery.js"></script> >> >> So Dromaeo will be added to PerformanceTests/Checkouts? Can we instead add it to Dromaeo/resources/? Or license doesn't allow it? > > Dromaeo specific test cases are MIT license. But I'm thinking we can check out as a part of test execution instead of checking it in statically. > If we're going to check it in, resource/ would be better.
I think we can check in MIT-license code:
https://lists.webkit.org/pipermail/webkit-dev/2010-January/011406.html
Hajime Morrita
Comment 7
2012-01-30 17:10:37 PST
Comment on
attachment 124524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124524&action=review
>>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 >>>> + re.compile(r'^CONSOLE MESSAGE: line \d+: \d+'), >>> >>> I'm not sure if we should ignore all console messages since some console messages contain actual errors. What are examples of console messages you get? >> >> I use console.log() to print progress for DRT. It's like "CONSOLE MESSAGE: Line 31: 3091". But I can remove this. > > I think we should remove that. There are cases where we want to report errors (e.g. script exceptions) using console message. You can call log(~) as well but I feel that console.log should be a stronger indication of something going wrong.
OK. I'll remove this.
>>>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:255 >>>> + re.compile(r'^Blocked access to external URL'), >>> >>> What are examples of external URLs Dromaeo tries to load? >> >> This is for icons which test html contains. For example:
https://github.com/omo/dromaeo/blob/master/tests/dom-query.html#L108
> > Huh... can we replace those urls by relative paths if we're checking tests into webkit?
Okay. I'll make some blank image and replace W3C logo with that.
>>>> PerformanceTests/Dromaeo/dom-query.html:3 >>>> +<script src="../Checkouts/dromaeo/web/jquery.js"></script> >>> >>> So Dromaeo will be added to PerformanceTests/Checkouts? Can we instead add it to Dromaeo/resources/? Or license doesn't allow it? >> >> Dromaeo specific test cases are MIT license. But I'm thinking we can check out as a part of test execution instead of checking it in statically. >> If we're going to check it in, resource/ would be better. > > I think we can check in MIT-license code:
https://lists.webkit.org/pipermail/webkit-dev/2010-January/011406.html
Sure. I found that it contains ext.js binary, which doesn't look capable for WebKit repo. I'll remove it before checkin. It should be done by
Bug 77328
.
Hajime Morrita
Comment 8
2012-01-30 18:41:57 PST
Created
attachment 124655
[details]
Patch
Ryosuke Niwa
Comment 9
2012-01-30 19:10:30 PST
Comment on
attachment 124655
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124655&action=review
> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 > + re.compile(r'has \d+ onunload handler')]
Is it possible to make this expression more explicit? We don't want to ignore random errors appearing on the same line (e.g. due to missing \n)
> PerformanceTests/ChangeLog:16 > + * Dromaeo/resources/drtrunner.js: Added.
It's kind of oxymoron to call it drtrunner.js now that this file is inside PerfomanceTests but I can't think of a better name.
Hajime Morrita
Comment 10
2012-01-30 21:07:02 PST
Comment on
attachment 124655
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=124655&action=review
>> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:254 >> + re.compile(r'has \d+ onunload handler')] > > Is it possible to make this expression more explicit? We don't want to ignore random errors appearing on the same line (e.g. due to missing \n)
Well, I'll put the exact message text here.
>> PerformanceTests/ChangeLog:16 >> + * Dromaeo/resources/drtrunner.js: Added. > > It's kind of oxymoron to call it drtrunner.js now that this file is inside PerfomanceTests but I can't think of a better name.
Good point. Will rename dromaeorunnner.js
Hajime Morrita
Comment 11
2012-01-30 22:48:59 PST
Created
attachment 124674
[details]
Patch for landing
Hajime Morrita
Comment 12
2012-01-30 22:50:10 PST
Comment on
attachment 124674
[details]
Patch for landing missing reviewer name.
Hajime Morrita
Comment 13
2012-01-30 22:51:13 PST
Created
attachment 124675
[details]
Patch for landing
WebKit Review Bot
Comment 14
2012-01-30 22:54:50 PST
Comment on
attachment 124675
[details]
Patch for landing Rejecting
attachment 124675
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ceTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file PerformanceTests/Dromaeo/dom-query.html patching file PerformanceTests/Dromaeo/resources/dromaeorunner.js patching file PerformanceTests/Skipped Hunk #1 FAILED at 6. 1 out of 1 hunk FAILED -- saving rejects to file PerformanceTests/Skipped.rej patching file PerformanceTests/resources/runner.js Hunk #1 succeeded at 63 with fuzz 1. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 Full output:
http://queues.webkit.org/results/11373171
Hajime Morrita
Comment 15
2012-01-31 02:01:18 PST
Created
attachment 124686
[details]
Patch for landing
WebKit Review Bot
Comment 16
2012-01-31 02:14:05 PST
Comment on
attachment 124686
[details]
Patch for landing Clearing flags on attachment: 124686 Committed
r106348
: <
http://trac.webkit.org/changeset/106348
>
WebKit Review Bot
Comment 17
2012-01-31 02:14:11 PST
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