Bug 172341 - Speedometer 2.0: jQuery and Inferno tests don't delete all ToDo items
Summary: Speedometer 2.0: jQuery and Inferno tests don't delete all ToDo items
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 172339
  Show dependency tree / graph
 
Reported: 2017-05-18 22:09 PDT by Ryosuke Niwa
Modified: 2017-05-19 02:43 PDT (History)
7 users (show)

See Also:

Attachments
Fixes the bug (3.90 KB, patch)
2017-05-18 22:14 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Added missing changes to InteractiveRunner.html (4.71 KB, patch)
2017-05-18 22:18 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2017-05-18 22:09:19 PDT 
There is a bug in the benchmark code and not all ToDo items are deleted in jQuery and inferno tests.
Comment 1 Ryosuke Niwa 2017-05-18 22:14:19 PDT 
Created attachment 310614 [details]
Fixes the bug
Comment 2 Ryosuke Niwa 2017-05-18 22:18:40 PDT 
Created attachment 310615 [details]
Added missing changes to InteractiveRunner.html
Comment 3 Antti Koivisto 2017-05-19 02:02:24 PDT 
Comment on attachment 310615 [details]
Added missing changes to InteractiveRunner.html

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

> PerformanceTests/Speedometer/resources/tests.js:343
> -            var checkboxes = contentDocument.querySelectorAll('.toggle');
> +            var checkboxes = contentDocument.getElementsByClassName('toggle');

Why the switch? You mostly use querySelector.
Comment 4 Ryosuke Niwa 2017-05-19 02:17:48 PDT 
Comment on attachment 310615 [details]
Added missing changes to InteractiveRunner.html

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

>> PerformanceTests/Speedometer/resources/tests.js:343
>> +            var checkboxes = contentDocument.getElementsByClassName('toggle');
> 
> Why the switch? You mostly use querySelector.

We have to get i-th element in the node list instead of the first.
So the alternative is to run querySelectorAll and get i-th element.
That's a lot less efficient.
Comment 5 WebKit Commit Bot 2017-05-19 02:43:43 PDT 
Comment on attachment 310615 [details]
Added missing changes to InteractiveRunner.html

Clearing flags on attachment: 310615

Committed r217112: <http://trac.webkit.org/changeset/217112>
Comment 6 WebKit Commit Bot 2017-05-19 02:43:45 PDT 
All reviewed patches have been landed.  Closing bug.