Add new test #2578

repasics · 2018-10-29T10:34:13Z

Added unusual-javascript-constructs.js to test corner cases

JerryScript-DCO-1.0-Signed-off-by: Csaba Repasi [email protected]

loki04

It looks fine by me.

loki04 · 2018-10-29T10:53:31Z

tests/jerry/unusual-javascript-constructs.js

+assert ([] + [] == "");
+assert ([] + {} == "[object Object]");
+//assert({} + [] == 0); // FIXME: assertion failed, it should be 0, but it is [object Object]
+//assert({} + {} == NaN); // // FIXME: assertion failed, it should be NaN, but it is [object Object][object Object]


Please create new issues for these two ones! It probably worth to add the link of the issues here as well.

There is no need to add new issues and also the assertions are wrong.

The correct asserts would look like:

assert (eval ("{} + []") == 0) assert (isNaN (eval ("{} + {}"))); // isNaN should be used instead of == NaN

The reason behind this is how {} is being parsed.

As a block (That's why {} + [] evaluates to 0, since {} evaluates to an empty block, and +[] gives 0 as a result)

As an object literal (putting this check in an assert the {} becomes a part of an expression so it's interpreted as an object)

Also these test cases should be added with a bit modification:

assert ({} + [] == "[object Object]") assert ({} + {} == "[object Object][object Object]");

This is strange. I've just built jerry and the interactive shell gives me:

jerry> {} + {} NaN

That's not [object Object][object Object].

Moreover, the assert couldn't be passing anyway as comparing anything to NaN (NaN included) gives false.

(To clarify: my above comment "This is strange" is not a response to @rerobika but a reaction to the commit itself. We must have been typing our reviews in parallel.)

@akosthekiss That's the expected behavior. Since you are using the shell, it evaluates the given line, so the output is correct. (same for the other test case)

jerry> {} + [] // parse {} as an empty block 0 jerry> print ( {} + [] ) // parse {} as an object literal ( now {} is a part of an expression) [object Object] undefined

akosthekiss

I've found lots of cases already being tested elsewhere. I'm not sure it's worth duplicating them.

akosthekiss · 2018-10-29T12:37:53Z

tests/jerry/unusual-javascript-constructs.js

+assert (x == !x);
+assert (Array(3) == ",,");
+
+assert (typeof NaN == "number");


this is being tested by test262/test/suite/ch08/8.5/S8.5_A3.js and test262/test/suite/ch15/15.1/15.1.1/15.1.1.1/S15.1.1.1_A1.js

I agree with you. If the test case exists elsewhere in the repository, there is no reason to duplicate it.

akosthekiss · 2018-10-29T12:40:46Z

tests/jerry/unusual-javascript-constructs.js

+assert (Array(3) == ",,");
+
+assert (typeof NaN == "number");
+assert (NaN != NaN);


(almost) this is being tested in test262/test/suite/ch11/11.9/11.9.2/S11.9.2_A4.1_T1.js and _T2.js (and the !== version at several other places)

akosthekiss · 2018-10-29T12:41:28Z

tests/jerry/unusual-javascript-constructs.js

+
+assert (typeof NaN == "number");
+assert (NaN != NaN);
+assert (typeof null == "object");


this is being tested in jerry/typeof.js

akosthekiss · 2018-10-29T12:43:19Z

tests/jerry/unusual-javascript-constructs.js

+assert (NaN != NaN);
+assert (typeof null == "object");
+assert (!(null instanceof Object));
+assert (!("string" instanceof String));


this is being tested in jerry-test-suite/11/11.08/11.08.06/11.08.06-004.js

akosthekiss · 2018-10-29T12:49:10Z

tests/jerry/unusual-javascript-constructs.js

+assert (9999999999999999 == 10000000000000000);
+assert (0.1 + 0.2 != 0.3);
+assert (!(Math.max() > Math.min()));
+assert (Math.max() < Math.min());


the expected values of Math.max() and Math.min() are being tested in jerry-test-suite/15/15.08/15.08.02/15.08.02.11/15.08.02.11-009.js and -010.js. -Infinity < Infinity is being tested in test262/test/suite/ch11/11.8/11.8.1/S11.8.1_A4.6.js

akosthekiss · 2018-10-29T12:50:29Z

tests/jerry/unusual-javascript-constructs.js

+//assert({} + [] == 0); // FIXME: assertion failed, it should be 0, but it is [object Object]
+//assert({} + {} == NaN); // // FIXME: assertion failed, it should be NaN, but it is [object Object][object Object]
+
+assert (true + true === 2);


this is being tested in test262/.../S11.6.1_A3.1_T1.1.js

akosthekiss · 2018-10-29T12:51:44Z

tests/jerry/unusual-javascript-constructs.js

+//assert({} + {} == NaN); // // FIXME: assertion failed, it should be NaN, but it is [object Object][object Object]
+
+assert (true + true === 2);
+assert (true - true === 0);


this is being tested in jerry-test-suite/.../11.06.02-010.js

akosthekiss · 2018-10-29T12:53:07Z

tests/jerry/unusual-javascript-constructs.js

+
+assert (true + true === 2);
+assert (true - true === 0);
+assert (!(true === 1));


this is being tested in test262/.../S11.9.4_A8_T1.js

akosthekiss · 2018-10-29T13:10:15Z

tests/jerry/unusual-javascript-constructs.js

+assert ([] + [] == "");
+assert ([] + {} == "[object Object]");
+//assert({} + [] == 0); // FIXME: assertion failed, it should be 0, but it is [object Object]
+//assert({} + {} == NaN); // // FIXME: assertion failed, it should be NaN, but it is [object Object][object Object]


This is strange. I've just built jerry and the interactive shell gives me:

jerry> {} + {} NaN

That's not [object Object][object Object].

Moreover, the assert couldn't be passing anyway as comparing anything to NaN (NaN included) gives false.

akosthekiss · 2018-10-29T13:10:49Z

tests/jerry/unusual-javascript-constructs.js

+
+assert ([] + [] == "");
+assert ([] + {} == "[object Object]");
+//assert({} + [] == 0); // FIXME: assertion failed, it should be 0, but it is [object Object]


This also seems to work for me in the interactive shell:

jerry> {} + [] 0

akosthekiss

My previous comments about most probably superfluous duplications of test cases have neither been answered nor actioned. But I see a new commit uploaded. This is a direct request now to remove the duplications.

akosthekiss · 2018-10-31T21:27:50Z

tests/jerry/unusual-javascript-constructs.js

+assert ({} + [] == "[object Object]");
+assert (isNaN (eval ("{} + {}")));
+
+assert ((! + [] + [] + ![]).length === 9);


This should rather be compared to "truefalse" instead of its length only.

akosthekiss · 2018-10-31T21:37:06Z

tests/jerry/unusual-javascript-constructs.js

+
+assert (!(null instanceof Object));
+
+assert (9999999999999999 == 10000000000000000);


this might be a better fit in tests/jerry/arithmetics-bignums.js (it already contains similar edge cases)

akosthekiss · 2018-10-31T21:45:43Z

tests/jerry/unusual-javascript-constructs.js

+assert (!(null instanceof Object));
+
+assert (9999999999999999 == 10000000000000000);
+assert (0.1 + 0.2 != 0.3);


this might go to tests/jerry/arithmetics-2.js which already contains a floating point imprecision test

akosthekiss · 2018-10-31T21:57:26Z

tests/jerry/unusual-javascript-constructs.js

+assert (eval ("{} + []") == 0);
+assert (isNaN (eval ("{} + {}")));
+assert ({} + [] == "[object Object]");
+assert (isNaN (eval ("{} + {}")));


something ain't right here.: {} + [] is tested with and without eval but {} + {} is tested with eval only (but twice).

akosthekiss · 2018-10-31T22:17:38Z

tests/jerry/unusual-javascript-constructs.js

+assert (x == !x);
+assert (Array(3) == ",,");
+
+assert (!(null instanceof Object));


Something like this is tested in tests/test262/test/suite/ch15/15.3/15.3.5/S15.3.5.3_A1_T6.js (testing [[HasInstance]]) but I agree that it's not completely the same. But then, this might deserve its own test case in tests/jerry-test-suite/11/11.08/11.08.06/ (tests of instanceof), a new 11.08.06-008.js test file.

akosthekiss · 2018-10-31T22:37:42Z

tests/jerry/unusual-javascript-constructs.js

@@ -0,0 +1,32 @@
+// Copyright JS Foundation and other contributors, http://js.foundation


"javascript" in the file name doesn't carry too much information (considering that this is the test suite of a javascript engine). I think tests/jerry/unusual.js would be enough.

tests/jerry-test-suite/11/11.08/11.08.06/11.08.06-008.js

akosthekiss · 2018-11-08T06:19:46Z

tests/jerry/arithmetics-2.js

@@ -28,6 +28,8 @@ assert(c == 210);
 c = a / b;
 assert(c >= 2.1 - 0.000001 && c <= 2.1 + 0.000001);

+assert (0.1 + 0.2 != 0.3);


Interesting choice to insert this in the middle of the a-b-c-related checks. Why not at the end of the file?

I've inserted it there because there is the only floating point check in the file. I tought it fits there.

I'd suggest moving it to the end.

tests/jerry-test-suite/11/11.08/11.08.06/11.08.06-008.js

Added tests/jerry/unusual.js to test corner cases Added tests/jerry-test-suite/11/11.08/11.08.06/11.08.06-008.js (a new test of instanceof) Added new floatingpoint test case to tests/jerry/arithmetics-2.js Added new test case to tests/jerry/arithmetics-bignums.js JerryScript-DCO-1.0-Signed-off-by: Csaba Repasi [email protected]

akosthekiss

LGTM

LaszloLango

LGTM

loki04 reviewed Oct 29, 2018

View reviewed changes

akosthekiss reviewed Oct 29, 2018

View reviewed changes

repasics force-pushed the special-cases branch from 3556220 to 8d584c2 Compare October 30, 2018 08:57

akosthekiss requested changes Oct 30, 2018

View reviewed changes

repasics force-pushed the special-cases branch from 8d584c2 to 67625e5 Compare October 31, 2018 08:05

akosthekiss requested changes Oct 31, 2018

View reviewed changes

akosthekiss added the test Related to testing label Oct 31, 2018

repasics force-pushed the special-cases branch from 67625e5 to 805a120 Compare November 5, 2018 10:33

rerobika reviewed Nov 7, 2018

View reviewed changes

tests/jerry-test-suite/11/11.08/11.08.06/11.08.06-008.js Show resolved Hide resolved

akosthekiss reviewed Nov 8, 2018

View reviewed changes

repasics force-pushed the special-cases branch from 805a120 to ca140d2 Compare November 9, 2018 08:05

akosthekiss approved these changes Nov 21, 2018

View reviewed changes

LaszloLango approved these changes Nov 22, 2018

View reviewed changes

akosthekiss merged commit 5533b8b into jerryscript-project:master Nov 22, 2018

zherczeg mentioned this pull request Mar 19, 2019

Nominating Robert Fancsik (@rerobika) to become a JerryScript reviewer #2799

Closed


		assert (!(null instanceof Object));

		assert (9999999999999999 == 10000000000000000);

		@@ -0,0 +1,32 @@
		// Copyright JS Foundation and other contributors, http://js.foundation

Add new test #2578

Add new test #2578

Uh oh!

Conversation

repasics commented Oct 29, 2018

Uh oh!

loki04 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rerobika Oct 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rerobika Oct 29, 2018 •

edited

Loading