[GeometryInterface] Clean-up layout tests for geometry interfaces.

[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None

Committed: https://crrev.com/33bb8aa1a9372768853a61717747e7551d9dbc54
Cr-Commit-Position: refs/heads/master@{#421272}

[Hwanseung Lee, 2016-09-25 13:01:47]

Description was changed from

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simply code using assert_*d_matrix_equals fuction.

BUG=None
==========

to

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simply code using assert_*d_matrix_equals fuction.

BUG=None
==========

[Hwanseung Lee, 2016-09-25 13:01:47]

hs1217.lee@samsung.com changed reviewers:
+ hs1217.lee@samsung.com, jinho.bang@samsung.com

[Hwanseung Lee, 2016-09-25 13:02:06]

hs1217.lee@samsung.com changed reviewers:
- hs1217.lee@samsung.com

[Hwanseung Lee, 2016-09-25 13:02:28]

@zino
PTAL. thank you.

[zino, 2016-09-25 15:01:40]

On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> @zino
> PTAL. thank you.

Looks good.
BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
assert_*d_matrix_equals)

[Hwanseung Lee, 2016-09-25 15:14:17]

On 2016/09/25 15:01:40, zino wrote:
> On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > @zino
> > PTAL. thank you.
> 
> Looks good.
> BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> assert_*d_matrix_equals)

yes. we can fix all layout tests for geometry interfaces.
i want to fix step by step per function.
do you want to fix all tests at this CL?

[Hwanseung Lee, 2016-09-25 15:18:47]

On 2016/09/25 15:14:17, Hwanseung Lee wrote:
> On 2016/09/25 15:01:40, zino wrote:
> > On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > > @zino
> > > PTAL. thank you.
> > 
> > Looks good.
> > BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> > assert_*d_matrix_equals)
> 
> yes. we can fix all layout tests for geometry interfaces.
> i want to fix step by step per function.
> do you want to fix all tests at this CL?

actually,
i think... after numberSequnce constructor CL merged, to fix all layout tests is
more simply.

[zino, 2016-09-25 15:58:37]

On 2016/09/25 15:14:17, Hwanseung Lee wrote:
> On 2016/09/25 15:01:40, zino wrote:
> > On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > > @zino
> > > PTAL. thank you.
> > 
> > Looks good.
> > BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> > assert_*d_matrix_equals)
> 
> yes. we can fix all layout tests for geometry interfaces.
> i want to fix step by step per function.
> do you want to fix all tests at this CL?

If so, please file a bug and use it.

[zino, 2016-09-25 16:13:38]

On 2016/09/25 15:58:37, zino wrote:
> On 2016/09/25 15:14:17, Hwanseung Lee wrote:
> > On 2016/09/25 15:01:40, zino wrote:
> > > On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > > > @zino
> > > > PTAL. thank you.
> > > 
> > > Looks good.
> > > BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> > > assert_*d_matrix_equals)
> > 
> > yes. we can fix all layout tests for geometry interfaces.
> > i want to fix step by step per function.
> > do you want to fix all tests at this CL?
> 
> If so, please file a bug and use it.

IMHO, we should fix all layout tests here. If not so, it will be difficult to
maintain the test codes.
You have already many pending CLs but they aren't easy to review on this
account.

My suggestion is that you should make all tests use assert_*d_matrix_equals()
first.
and then we can land your sequence constructor patch.
Then, we should make all tests use sequence constructor.

What do you think?

[Hwanseung Lee, 2016-09-25 23:10:50]

On 2016/09/25 16:13:38, zino wrote:
> On 2016/09/25 15:58:37, zino wrote:
> > On 2016/09/25 15:14:17, Hwanseung Lee wrote:
> > > On 2016/09/25 15:01:40, zino wrote:
> > > > On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > > > > @zino
> > > > > PTAL. thank you.
> > > > 
> > > > Looks good.
> > > > BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> > > > assert_*d_matrix_equals)
> > > 
> > > yes. we can fix all layout tests for geometry interfaces.
> > > i want to fix step by step per function.
> > > do you want to fix all tests at this CL?
> > 
> > If so, please file a bug and use it.
> 
> IMHO, we should fix all layout tests here. If not so, it will be difficult to
> maintain the test codes.
> You have already many pending CLs but they aren't easy to review on this
> account.
> 
> My suggestion is that you should make all tests use assert_*d_matrix_equals()
> first.
> and then we can land your sequence constructor patch.
> Then, we should make all tests use sequence constructor.
> 
> What do you think?

um.. i agree
but assert_*d_matrix_equals() have problem to distinguish +0 and -0.
(https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/resources...)
when execute some function(ex. invertSelf()) in 2d matrix, m13 have -0 really,
but expected vaule of m13 is 0 in assert_2d_matrix_equals()
(https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/dom/...)

need to distinguish +0 and -0 at test?
if needed it at test, we should fix some function(ex invert*(), flip*())

[Hwanseung Lee, 2016-09-25 23:17:20]

On 2016/09/25 23:10:50, Hwanseung Lee wrote:
> On 2016/09/25 16:13:38, zino wrote:
> > On 2016/09/25 15:58:37, zino wrote:
> > > On 2016/09/25 15:14:17, Hwanseung Lee wrote:
> > > > On 2016/09/25 15:01:40, zino wrote:
> > > > > On 2016/09/25 13:02:28, Hwanseung Lee wrote:
> > > > > > @zino
> > > > > > PTAL. thank you.
> > > > > 
> > > > > Looks good.
> > > > > BTW, Shouldn't we fix all layout tests for geometry interfaces? (Using
> > > > > assert_*d_matrix_equals)
> > > > 
> > > > yes. we can fix all layout tests for geometry interfaces.
> > > > i want to fix step by step per function.
> > > > do you want to fix all tests at this CL?
> > > 
> > > If so, please file a bug and use it.
> > 
> > IMHO, we should fix all layout tests here. If not so, it will be difficult
to
> > maintain the test codes.
> > You have already many pending CLs but they aren't easy to review on this
> > account.
> > 
> > My suggestion is that you should make all tests use
assert_*d_matrix_equals()
> > first.
> > and then we can land your sequence constructor patch.
> > Then, we should make all tests use sequence constructor.
> > 
> > What do you think?
> 
> um.. i agree
> but assert_*d_matrix_equals() have problem to distinguish +0 and -0.
>
(https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/resources...)
> when execute some function(ex. invertSelf()) in 2d matrix, m13 have -0 really,
> but expected vaule of m13 is 0 in assert_2d_matrix_equals()
>
(https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/dom/...)
> 
> need to distinguish +0 and -0 at test?
> if needed it at test, we should fix some function(ex invert*(), flip*())

or if don't need compare unused element(ex. m13, m14, m23, ...) at 2d matrix, we
should fix testhelper.js.

[Hwanseung Lee, 2016-09-26 13:27:28]

@zino
PTAL.
first all, i did fix all layout test using assert_*d_matrix_equals function.
thank you.

[Hwanseung Lee, 2016-09-26 13:33:57]

Description was changed from

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simply code using assert_*d_matrix_equals fuction.

BUG=None
==========

to

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simply test code using assert_*d_matrix_equals function.

BUG=None
==========

[Hwanseung Lee, 2016-09-26 13:34:24]

Description was changed from

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simply test code using assert_*d_matrix_equals function.

BUG=None
==========

to

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simple test code using assert_*d_matrix_equals function.

BUG=None
==========

[zino, 2016-09-27 16:31:19]

https://codereview.chromium.org/2370623003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(left):

https://codereview.chromium.org/2370623003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:62:
function assert_matrix_equals(actual, expected, description) {
Do we still need description?

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(left):

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:22:
if (Array.isArray(expected)) {
assert_true(Array.isArray(expected));

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:45:
if (Array.isArray(expected)) {
assert_true(Array.isArray(expected));

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(right):

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:1:
function assert_identity_2d_matrix(actual) {
assert_true(actual instanceof DOMMatrixReadOnly);

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:4:
assert_array_equals(actual.toFloat64Array(), [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1,
0, 0, 0, 0, 1]);
Let's make assert_identity_matrix(actual)

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:8:
assert_false(actual.is2D);
These assert functions should have description?

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:15:
for (var i = array.length - 1; i >= 0; i--) {
// Do not care negative zero for testing accomodation.
for (var i = 0; i < array.length; i++) {
  if (array[i] === -0)
    array[i] = 0;
}

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:23:
function assert_2d_matrix_equals(actual, expected) {
assert_true(actual instanceof DOMMatrixReadOnly);

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:37:
function assert_3d_matrix_equals(actual, expected) {
assert_true(actual instanceof DOMMatrixReadOnly);

[zino, 2016-09-27 16:31:25]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 16:32:14]

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

[zino, 2016-09-27 16:43:45]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 16:44:25]

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

[Hwanseung Lee, 2016-09-27 16:44:31]

@zino.
PTAL.
i did fix a some code following your guide.
thank you.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(right):

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:1:
function assert_identity_2d_matrix(actual) {
On 2016/09/27 16:31:18, zino wrote:
> assert_true(actual instanceof DOMMatrixReadOnly);

Done.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:4:
assert_array_equals(actual.toFloat64Array(), [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1,
0, 0, 0, 0, 1]);
On 2016/09/27 16:31:19, zino wrote:
> Let's make assert_identity_matrix(actual)

Done.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:8:
assert_false(actual.is2D);
On 2016/09/27 16:31:19, zino wrote:
> These assert functions should have description?

Done.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:15:
for (var i = array.length - 1; i >= 0; i--) {
On 2016/09/27 16:31:19, zino wrote:
> // Do not care negative zero for testing accomodation.
> for (var i = 0; i < array.length; i++) {
>   if (array[i] === -0)
>     array[i] = 0;
> }

Done.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:23:
function assert_2d_matrix_equals(actual, expected) {
On 2016/09/27 16:31:19, zino wrote:
> assert_true(actual instanceof DOMMatrixReadOnly);

Done.

https://codereview.chromium.org/2370623003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:37:
function assert_3d_matrix_equals(actual, expected) {
On 2016/09/27 16:31:18, zino wrote:
> assert_true(actual instanceof DOMMatrixReadOnly);

Done.

[zino, 2016-09-27 16:47:49]

https://codereview.chromium.org/2370623003/diff/150012/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(right):

https://codereview.chromium.org/2370623003/diff/150012/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:23:
if (array[i] === -0)
nit: indentation

[Hwanseung Lee, 2016-09-27 16:53:36]

@zino
i did fix a indentation.
PTAL. thank you.

https://codereview.chromium.org/2370623003/diff/150012/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js
(right):

https://codereview.chromium.org/2370623003/diff/150012/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/dom/resources/geometry-interfaces-test-helpers.js:23:
if (array[i] === -0)
On 2016/09/27 16:47:49, zino wrote:
> nit: indentation

Done.

[Hwanseung Lee, 2016-09-27 17:16:15]

Description was changed from

==========
[GeometryInterface] Fix a indentation and clean up.

some lines has a 4 space. but other line has a 2 space.
so change to a 2 space for indentatin in order to consistent.
and make more simple test code using assert_*d_matrix_equals function.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

helper methods receive array as parameter of expected
instead of dictionary at test_helps.js.
and clean up test code using modified methods.
also fix an indentation.

BUG=None
==========

[Hwanseung Lee, 2016-09-27 17:17:09]

Description was changed from

==========
[GeometryInterface] clean up test file.

helper methods receive array as parameter of expected
instead of dictionary at test_helps.js.
and clean up test code using modified methods.
also fix an indentation.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

helper methods take array as parameter of expected
instead of dictionary at test_helps.js.
and clean up test code using modified methods.
also fix an indentation.

BUG=None
==========

[Hwanseung Lee, 2016-09-27 17:19:00]

Description was changed from

==========
[GeometryInterface] clean up test file.

helper methods take array as parameter of expected
instead of dictionary at test_helps.js.
and clean up test code using modified methods.
also fix an indentation.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

Make the assert functions take array instead of dictionary.
also fix wrong indentation in test codes.

BUG=None
==========

[Hwanseung Lee, 2016-09-27 17:19:38]

Description was changed from

==========
[GeometryInterface] clean up test file.

Make the assert functions take array instead of dictionary.
also fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

Make the assert functions take array instead of dictionary.
Also, fix wrong indentation in test codes.

BUG=None
==========

[Hwanseung Lee, 2016-09-27 17:21:17]

Description was changed from

==========
[GeometryInterface] clean up test file.

Make the assert functions take array instead of dictionary.
Also, fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

The assert functions take array as expected argument instead of dictionary at
helper.js
Also, fix wrong indentation in test codes.

BUG=None
==========

[Hwanseung Lee, 2016-09-27 17:21:30]

Description was changed from

==========
[GeometryInterface] clean up test file.

The assert functions take array as expected argument instead of dictionary at
helper.js
Also, fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] clean up test file.

The assert functions take array as expected argument
 instead of dictionary at helper.js.
Also, fix wrong indentation in test codes.

BUG=None
==========

[zino, 2016-09-27 17:24:09]

Description was changed from

==========
[GeometryInterface] clean up test file.

The assert functions take array as expected argument
 instead of dictionary at helper.js.
Also, fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None
==========

[zino, 2016-09-27 17:25:12]

lgtm

[Hwanseung Lee, 2016-09-27 17:27:02]

The CQ bit was checked by hs1217.lee@samsung.com

[commit-bot: I haz the power, 2016-09-27 17:27:31]

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-27 18:20:44]

Description was changed from

==========
[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None
==========

[commit-bot: I haz the power, 2016-09-27 18:20:46]

Committed patchset #10 (id:170001)

[commit-bot: I haz the power, 2016-09-27 18:22:58]

Description was changed from

==========
[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None
==========

to

==========
[GeometryInterface] Clean-up layout tests for geometry interfaces.

Make the assert functions take array instead of dictionary as expected argument.
Also, fix wrong indentation in test codes.

BUG=None

Committed: https://crrev.com/33bb8aa1a9372768853a61717747e7551d9dbc54
Cr-Commit-Position: refs/heads/master@{#421272}
==========

[commit-bot: I haz the power, 2016-09-27 18:22:59]

Patchset 10 (id:??) landed as
https://crrev.com/33bb8aa1a9372768853a61717747e7551d9dbc54
Cr-Commit-Position: refs/heads/master@{#421272}