Bug 77329

Summary:

[PerformanceTests] Add landing html for Dromaeo dom-query test

Product:

WebKit

Reporter:

Hajime Morrita <morrita>

Component:

Tools / Tests

Assignee:

Hajime Morrita <morrita>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ojan, rniwa, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

76156

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none
Patch for landing	none
Patch for landing	none

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.