Increase test coverage: Array.prototype.lastIndexOf #2688

matedabis · 2019-01-09T14:28:01Z

Branch coverage:
Before: 32/40
After: 40/40

JerryScript-DCO-1.0-Signed-off-by: Mate Dabis [email protected]

LaszloLango · 2019-01-09T14:40:25Z

tests/jerry/array-prototype-lastindexof.js

+} catch (e) { }
+
+// Checking behavior when length is 0
+try{


Missing space before {

LaszloLango · 2019-01-09T14:40:47Z

tests/jerry/array-prototype-lastindexof.js

+
+// Checking behavior when the starting index is out of the range of the original array, so it points
+// to an empty space, as we modified the length of the array before
+try{


Missing space before {

LaszloLango · 2019-01-09T14:41:03Z

tests/jerry/array-prototype-lastindexof.js

+
+// Checking behavior when the 3rd argument (start index) is greater than the length of the first argument
+try {
+  Array.prototype.lastIndexOf.call("foo", "foo", 999)


Missing semicolon

LaszloLango · 2019-01-09T14:41:07Z

tests/jerry/array-prototype-lastindexof.js

+try {
+  var o = {};
+  Object.defineProperty(o, 'toString', { 'get' : function () { throw new ReferenceError ("foo"); } });
+  Array.prototype.lastIndexOf.call("foo", "foo", o)


Missing semicolon

LaszloLango · 2019-01-09T14:41:10Z

tests/jerry/array-prototype-lastindexof.js

+  var o = {};
+  Object.defineProperty(o, 'toString', { 'get' : function () { throw new ReferenceError ("foo"); } });
+  var a = { length : o }
+  Array.prototype.lastIndexOf.call(a, function () { })


Missing semicolon

LaszloLango · 2019-01-09T14:41:43Z

tests/jerry/array-prototype-lastindexof.js

+  Object.defineProperty(o, 'toString', { 'get' : function () { throw new ReferenceError ("foo"); } });
+  var a = { length : o }
+  Array.prototype.lastIndexOf.call(a, function () { })
+  assert (false);


no space needed before (

LaszloLango · 2019-01-09T14:41:50Z

tests/jerry/array-prototype-lastindexof.js

+  Array.prototype.lastIndexOf.call(a, function () { })
+  assert (false);
+} catch (e) {
+  assert (e instanceof ReferenceError);


no space needed before (

matedabis · 2019-01-10T07:54:34Z

@LaszloLango
Fixed.

LaszloLango

LGTM

rerobika

Please apply the suggestions for all the new related test cases.

rerobika · 2019-01-10T08:09:50Z

tests/jerry/array-prototype-lastindexof.js

+// Checking behavior when there are no arguments except "this"
+try {
+  var a = "This is a sample text string to test this function";
+  Array.prototype.lastIndexOf.call(a);


Add an assert(false) after this line, to ensure that an exception has been thrown.

rerobika · 2019-01-10T08:10:25Z

tests/jerry/array-prototype-lastindexof.js

+try {
+  var a = "This is a sample text string to test this function";
+  Array.prototype.lastIndexOf.call(a);
+} catch (e) {}


Add an assert for e instanceof TypeError to validate the exception.

matedabis · 2019-01-10T09:47:09Z

Corrected

rerobika · 2019-01-11T11:04:34Z

tests/jerry/array-prototype-lastindexof.js

+
+// Checking behavior when there are no arguments except "this"
+var a = "This is a sample text string to test this function";
+Array.prototype.lastIndexOf.call(a);


Since it does not throw exception, add an assert for the expected result.

rerobika · 2019-01-11T11:04:50Z

tests/jerry/array-prototype-lastindexof.js

+Array.prototype.lastIndexOf.call(a);
+
+// Checking behavior when value is null
+try{


Space before {.

rerobika · 2019-01-11T11:04:58Z

tests/jerry/array-prototype-lastindexof.js

+}
+
+// Checking behavior when length is 0
+Array.prototype.lastIndexOf.call("", "chocolate cake");


rerobika · 2019-01-11T11:05:09Z

tests/jerry/array-prototype-lastindexof.js

+}
+
+// Checking behavior when the 3rd argument (start index) is greater than the length of the first argument
+Array.prototype.lastIndexOf.call("foo", "foo", 999);


rerobika · 2019-01-11T11:05:18Z

tests/jerry/array-prototype-lastindexof.js

+// to an empty space, as we modified the length of the array before
+var a = [1, 2, 3];
+Object.defineProperty(a, "length", {value: 10});
+Array.prototype.lastIndexOf.call(a, "", 6);


rerobika · 2019-01-11T11:06:26Z

tests/jerry/array-prototype-lastindexof.js

+  Array.prototype.lastIndexOf.call(a, function () { });
+  assert(false);
+} catch (e) {
+  assert(e instanceof ReferenceError);


Add an assert for e.message === "foo"'.

rerobika · 2019-01-11T11:06:34Z

tests/jerry/array-prototype-lastindexof.js

+  Array.prototype.lastIndexOf.call("foo", "foo", o);
+  assert(false);
+} catch (e) {
+  assert(e instanceof ReferenceError);


Same as above.

Branch coverage: Before: 32/40 After: 40/40 JerryScript-DCO-1.0-Signed-off-by: Mate Dabis [email protected]

matedabis · 2019-01-14T07:48:12Z

Corrected.

akosthekiss · 2019-01-14T17:28:10Z

@matedabis Could you please give details similar to those requested at #2674 (comment) . I.e., which source file(s) or source file entity(-ies) (function(s)) are investigated, and what is the coverage result when all test suites of the project are executed (i.e., what is the actual baseline)?

matedabis · 2019-01-15T08:00:14Z

@akosthekiss
The function we are testing: ecma_builtin_array_prototype_object_splice
The test file: /tests/jerry/array-prototype-splice.js
I dont know yet the result if all test suites are executed. By all test suites of the project do you mean all the tests in run-tests.py or all the .js files in the tests folder, or both?

zherczeg

LGTM

akosthekiss

I'd like to have this on hold until validity questions are sorted out at #2674 .

matedabis · 2019-01-17T13:53:37Z

Closing pull request because the updated version of the coverage tester script wich runs --jerry-test-suite --test262 --unittests --jerry-tests tests, shows that the branches of this function are already covered so these changes are not needed.
The script:
https://github.com/matedabis/jerryscript/blob/gcov_coverage_tester/tests/gcov-tests/gcovtester.py

LaszloLango requested changes Jan 9, 2019

View reviewed changes

matedabis force-pushed the last_index_of branch from 83cd17d to 864c6fa Compare January 10, 2019 07:50

LaszloLango approved these changes Jan 10, 2019

View reviewed changes

rerobika requested changes Jan 10, 2019

View reviewed changes

matedabis force-pushed the last_index_of branch 2 times, most recently from d35de5c to 1fd95d5 Compare January 10, 2019 09:46

rerobika requested changes Jan 11, 2019

View reviewed changes

Increase test coverage: Array.prototype.lastIndexOf

22c7af9

Branch coverage: Before: 32/40 After: 40/40 JerryScript-DCO-1.0-Signed-off-by: Mate Dabis [email protected]

matedabis force-pushed the last_index_of branch from 1fd95d5 to 22c7af9 Compare January 14, 2019 07:48

zherczeg approved these changes Jan 15, 2019

View reviewed changes

akosthekiss requested changes Jan 17, 2019

View reviewed changes

matedabis closed this Jan 17, 2019

zherczeg mentioned this pull request Mar 19, 2019

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

Closed

Increase test coverage: Array.prototype.lastIndexOf #2688

Increase test coverage: Array.prototype.lastIndexOf #2688

Uh oh!

Conversation

matedabis commented Jan 9, 2019

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!

matedabis commented Jan 10, 2019

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

rerobika 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!

matedabis commented Jan 10, 2019

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!

matedabis commented Jan 14, 2019

Uh oh!

akosthekiss commented Jan 14, 2019

Uh oh!

matedabis commented Jan 15, 2019

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

matedabis commented Jan 17, 2019

Uh oh!

Uh oh!