Convert LayoutTests/svg/dom/SVGLengthList*.html js-tests.js to testharness.js based tests

Convert LayoutTests/svg/dom/SVGLengthList*.html js-tests.js to testharness.js based tests

BUG=636710
Committed: https://crrev.com/59a29f5a769c4ebf7c024a1c74a6ac22814b86f8
Cr-Commit-Position: refs/heads/master@{#419421}

[Shanmuga Pandi, 2016-09-17 11:09:44]

shanmuga.m@samsung.com changed reviewers:
+ srirama.m@samsung.com

[Shanmuga Pandi, 2016-09-17 11:09:44]

PTAL!!
Thanks

[Srirama, 2016-09-17 11:27:53]

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:28:
assert_equals(newLength1 != newLength2, true);
use assert_not_equals here and below.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:31:
assert_equals(newLength1.value == newLength2.value - 50, true);
assert_equals(newLength1.value, newLength2.value - 50);

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:32:
assert_equals(newLength2.value + 50 == newLength3.value, true);
modify this as well.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:13: //
Check initial list state of text");
nit: remove "); at the end of comment.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:2:
<title>SVGLengthList, reacts to XML DOM modifications</title>
nit: s/reacts/reaction

[Shanmuga Pandi, 2016-09-17 12:29:34]

PTAL

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:28:
assert_equals(newLength1 != newLength2, true);
On 2016/09/17 11:27:52, Srirama wrote:
> use assert_not_equals here and below.

Done.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:31:
assert_equals(newLength1.value == newLength2.value - 50, true);
On 2016/09/17 11:27:52, Srirama wrote:
> assert_equals(newLength1.value, newLength2.value - 50);

Done.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:32:
assert_equals(newLength2.value + 50 == newLength3.value, true);
On 2016/09/17 11:27:52, Srirama wrote:
> modify this as well.

Done.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:13: //
Check initial list state of text");
On 2016/09/17 11:27:53, Srirama wrote:
> nit: remove "); at the end of comment.

Done.

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html
(right):

https://codereview.chromium.org/2344403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:2:
<title>SVGLengthList, reacts to XML DOM modifications</title>
On 2016/09/17 11:27:53, Srirama wrote:
> nit: s/reacts/reaction

Done.

[Srirama, 2016-09-17 12:48:40]

srirama.m@samsung.com changed reviewers:
+ fs@opera.com

[Srirama, 2016-09-17 12:48:41]

LGTM with a suggestion

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:35:
assert_equals(list1.numberOfItems, 3);
this checking looks common in most of the places, so may be we can write a
helper function by passing list and expected value array.
Something like
function assert_list(list, expectedValues) {
  assert_equals(list.numberOfItems, expectedValues.length);
  for (var index of expectedValues)
    assert_equals(list.getItem(index).value, expectedValues[index]);

  assert_throws("IndexSizeError", function() { list1.getItem(++index); });
}

I will leave it to fs@ to have final say.

[fs, 2016-09-17 17:42:37]

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html:13: // This
is a test of the SVGLengthList::initialize() API.
Nit: :: -> .

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html:72: //
Assure that the 'itemInAnotherList' item is still live anymore, set value to 999
then back to 50.
This comment sounds funny. Could you change to something less cryptic? Maybe:
"Ensure that 'itemInAnotherList' isn't live (wrt 'list3') by changing its
value."

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:10:
// This is a test of the SVGLengthList::insertItemBefore() API.
Like in previous test.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:51:
// Storing getItem(0/1/2) in local variables.
0/1/2 -> 0/1/2/3

?

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:10: // This
is a test of the SVGLengthList::removeItem() API.
Like in previous test(s)

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:22: //
Remove fourth item of text.
s/fourth/first/? (Can probably drop "of text" too.)

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:13: //
This is a test of the SVGLengthList::replaceItem() API.
Like...

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:35:
assert_equals(list1.numberOfItems, 3);
On 2016/09/17 at 12:48:41, Srirama wrote:
> this checking looks common in most of the places, so may be we can write a
helper function by passing list and expected value array.
> Something like
> function assert_list(list, expectedValues) {
>   assert_equals(list.numberOfItems, expectedValues.length);
>   for (var index of expectedValues)
>     assert_equals(list.getItem(index).value, expectedValues[index]);
> 
>   assert_throws("IndexSizeError", function() { list1.getItem(++index); });
> }
> 
> I will leave it to fs@ to have final say.

Yes, that sounds like a good idea. Would cut down quite a bit on boilerplate.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:108: //
Replace the second item in text4 x list with the second item in the text4 x
list.
...with the third item in the text3 x list.

?

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:2:
<title>SVGLengthList, reaction to XML DOM modifications</title>
I don't see anything particularly "XML" about this test (it's now even in
HTML...), so drop "XML" here and elsewhere (and from the filename.)

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:16:
var text1XBaseValGetItem0 = list.getItem(0);
Maybe just rename these to "item0" (etc).

[Shanmuga Pandi, 2016-09-19 07:40:04]

PTAL!
Thank you

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html:13: // This
is a test of the SVGLengthList::initialize() API.
On 2016/09/17 17:42:36, fs wrote:
> Nit: :: -> .

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-initialize.html:72: //
Assure that the 'itemInAnotherList' item is still live anymore, set value to 999
then back to 50.
On 2016/09/17 17:42:36, fs wrote:
> This comment sounds funny. Could you change to something less cryptic? Maybe:
> "Ensure that 'itemInAnotherList' isn't live (wrt 'list3') by changing its
> value."

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:10:
// This is a test of the SVGLengthList::insertItemBefore() API.
On 2016/09/17 17:42:36, fs wrote:
> Like in previous test.

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-insertItemBefore.html:51:
// Storing getItem(0/1/2) in local variables.
On 2016/09/17 17:42:37, fs wrote:
> 0/1/2 -> 0/1/2/3
> 
> ?

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:10: // This
is a test of the SVGLengthList::removeItem() API.
On 2016/09/17 17:42:37, fs wrote:
> Like in previous test(s)

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-removeItem.html:22: //
Remove fourth item of text.
On 2016/09/17 17:42:37, fs wrote:
> s/fourth/first/? (Can probably drop "of text" too.)

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:13: //
This is a test of the SVGLengthList::replaceItem() API.
On 2016/09/17 17:42:37, fs wrote:
> Like...

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:35:
assert_equals(list1.numberOfItems, 3);
On 2016/09/17 17:42:37, fs wrote:
> On 2016/09/17 at 12:48:41, Srirama wrote:
> > this checking looks common in most of the places, so may be we can write a
> helper function by passing list and expected value array.
> > Something like
> > function assert_list(list, expectedValues) {
> >   assert_equals(list.numberOfItems, expectedValues.length);
> >   for (var index of expectedValues)
> >     assert_equals(list.getItem(index).value, expectedValues[index]);
> > 
> >   assert_throws("IndexSizeError", function() { list1.getItem(++index); });
> > }
> > 
> > I will leave it to fs@ to have final say.
> 
> Yes, that sounds like a good idea. Would cut down quite a bit on boilerplate.

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:108: //
Replace the second item in text4 x list with the second item in the text4 x
list.
On 2016/09/17 17:42:37, fs wrote:
> ...with the third item in the text3 x list.
> 
> ?

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html
(right):

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:2:
<title>SVGLengthList, reaction to XML DOM modifications</title>
On 2016/09/17 17:42:37, fs wrote:
> I don't see anything particularly "XML" about this test (it's now even in
> HTML...), so drop "XML" here and elsewhere (and from the filename.)

Done.

https://codereview.chromium.org/2344403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-xml-dom-modifications.html:16:
var text1XBaseValGetItem0 = list.getItem(0);
On 2016/09/17 17:42:37, fs wrote:
> Maybe just rename these to "item0" (etc).

Done.

[fs, 2016-09-19 08:06:43]

lgtm

https://codereview.chromium.org/2344403002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html
(right):

https://codereview.chromium.org/2344403002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:12:
function assert_list(list, expectedValues) {
Could you move this to a helper .js file and use it in all these tests? (Or
maybe not all, but most could make good use of this helper.) Perfectly ok to do
that as a follow-up.

[Shanmuga Pandi, 2016-09-19 08:50:35]

On 2016/09/19 08:06:43, fs wrote:
> lgtm
> 
>
https://codereview.chromium.org/2344403002/diff/40001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html
> (right):
> 
>
https://codereview.chromium.org/2344403002/diff/40001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/svg/dom/SVGLengthList-replaceItem.html:12:
> function assert_list(list, expectedValues) {
> Could you move this to a helper .js file and use it in all these tests? (Or
> maybe not all, but most could make good use of this helper.) Perfectly ok to
do
> that as a follow-up.

Thanks fs@.
I will do as follow up patch.

[Shanmuga Pandi, 2016-09-19 08:50:56]

The CQ bit was checked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2016-09-19 08:50:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
srirama.m@samsung.com
Link to the patchset: https://codereview.chromium.org/2344403002/#ps40001
(title: "Align with review comments")

[commit-bot: I haz the power, 2016-09-19 08:51:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 09:54:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-19 09:56:33]

Description was changed from

==========
Convert LayoutTests/svg/dom/SVGLengthList*.html js-tests.js to testharness.js
based tests

BUG=636710
==========

to

==========
Convert LayoutTests/svg/dom/SVGLengthList*.html js-tests.js to testharness.js
based tests

BUG=636710

Committed: https://crrev.com/59a29f5a769c4ebf7c024a1c74a6ac22814b86f8
Cr-Commit-Position: refs/heads/master@{#419421}
==========

[commit-bot: I haz the power, 2016-09-19 09:56:34]

Patchset 3 (id:??) landed as
https://crrev.com/59a29f5a769c4ebf7c024a1c74a6ac22814b86f8
Cr-Commit-Position: refs/heads/master@{#419421}