Increase test coverage: Array.prototype.sort

[repasics]

Added new test cases to array-prototype-sort.js to improve branch coverage.
Branch coverage:
sort:
- before: 44/50
- after: 48/48

sort compare helper:
- before: 18/24
- after: 24/24

Also removed an unnecessary condition from the while loop what counts properties
with lower index than len.

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

[akosthekiss]

@repasics: How did you measure branch coverage? Which tool did you use?

[matedabis]

@akosthekiss
We used LCOV to measure the coverage.

[rerobika]

These two lines are misaligned.

[akosthekiss]

@repasics @matedabis Could you please elaborate on the whole measurement setup a bit more? How did you build jerryscript, how did you invoke lcov, which test suite(s) did you execute?

[matedabis]

To set up LCOV I used the help of the following page:
http://ltp.sourceforge.net/coverage/lcov.php
There is also an example for how the result looks like.

We used this script I made to measure coverage:
https://github.com/matedabis/jerryscript/blob/gcov_coverage_tester/tests/gcov-tests/gcovtester.py

JerryScript build command:
https://github.com/matedabis/jerryscript/blob/6a2b9cc3ce2e6621b133fc5003309ad160cc0a0e/tests/gcov-tests/gcovtester.py#L47

Coverage information capture command:
https://github.com/matedabis/jerryscript/blob/6a2b9cc3ce2e6621b133fc5003309ad160cc0a0e/tests/gcov-tests/gcovtester.py#L75

Generate HTML command:
https://github.com/matedabis/jerryscript/blob/6a2b9cc3ce2e6621b133fc5003309ad160cc0a0e/tests/gcov-tests/gcovtester.py#L88

It is also important, that in ECMA_TRY_CATCH and ECMA_OP_TO_NUMBER_TRY_CATCH we had to disable the following line:
JERRY_ASSERT (return_value == ECMA_VALUE_EMPTY);
Because of more than one test cases, if it is enabled, we may can not reach some branches.

[akosthekiss]

@matedabis Thanks for the link. The script and your note raise some further questions:

• The script runs only a single test file in the engine (via the command line shell). Coverage (be that line or branch coverage) is usually measured by running all tests in a test suite or by running all available test suites. So, what are the (branch) coverage results when all test suites are executed?
• What is 48? Total number of branches of a function? Of a file? Which function? Which file? The script you linked or the commit message above don't answer that.
• The removal of an assert is a red flag to me. What branches can only be reached if an assert is removed? Asserts should not make any difference (theoretically, at least).

[matedabis]

@akosthekiss

1. I don't know that yet, we only measured the coverage of functions each with its test file in the /tests/jerry folder. For example in this case we test ecma_builtin_array_prototype_object_sort function only with /tests/jerry/array-prototype-sort.js. But if it is better, we will test the coverage by running all the test suites.

2. The total number is the number of branches of the function we are actually testing, in this case, the mentioned above, excluding JERRY_ASSERTs.

3. The reason we do not test JERRY_ASSERTs and we removed 1-1 of them in ECMA_TRY_CATCH and ECMA_OP_TO_NUMBER_TRY_CATCH is that, there are many test cases in a file, and if we trigger an assert, the engine stops running the js code at that point, so the test cases below are not executed. This way if we trigger one of the try-catches, the code does not stop running, we can reach those branches without disabling the asserts, but we would have this issue. This way I think we have two options. Either splitting the test file to many small parts to avoid this problem, or do not trigger asserts, and disable asserts in try-catches.

[akosthekiss]

1. I'm a bit puzzled as I recall we've had similar discussions before when the test suite was planned to get extended with additional code. Then, I've pointed at both the jerry-test-suite and at test262. Picking out single test files from a suite can be quite misleading about the coverage. At the least, /tests/jerry-test-suite and /tests/jerry should be executed before drawing any conclusions, as they are project internal suites. But I'd be interested about combining them with test262, too, since that's a submodule, too.

2. I cannot get my head around two things. First, in a well-behaving engine, asserts must not happen in the executed test files. No matter how many test cases are written in a single .js file, they will all run. If they didn't execute properly, Travis CI were red. Second, what's the connection between asserts in .js test files and asserts in C macros? I cannot see how they affect each other.

[matedabis]

Sorry I realized I misunderstood a thing, I did not want to mislead you. JERRY_ASSERTs are only disabled to see the real branch coverage, without the asserts. Of course we can reach all branches with asserts enabled, except 1 branch of each asserts. It is only for better visibility.

[matedabis]

I will create a modified version of the coverage tester script and test the coverage with the test suites.

[akosthekiss]

re. asserts: If I get it correctly, what you'd like to achieve is to disable asserts in debug builds. In that case you should disable JERRY_ASSERT in jerry-core/jrt/jrt.h, not its call sites all over the code base (e.g., in ECMA_TRY_CATCH or ECMA_OP_TO_NUMBER_TRY_CATCH)

[matedabis]

Yes, that is what I would like to achieve. That will help for sure, thank you.

[repasics]

I checked the coverage including jerry-test-suite and test262 tests, then removed the unnecessary test cases.
The new stat is:
- before: 44/50
- after: 48/48
The two branch difference between the two stats is because the removed condition from the while loop what counts properties with lower index than len.

[rerobika]

LGTM (informal)

After the #2721 statistic this PR should contain the tests for the sort_compare_helper function as well to achive the full branch coverage.

[repasics]

I added new test cases what covers the sort_compare_helper function too.

[repasics]

Summary:
The following script is made for testing branch coverage with all the test suites (--jerry-test-suite --test262 --unittests --jerry-tests), or with only one .js file.

https://github.com/matedabis/jerryscript/blob/gcov_coverage_tester/tests/gcov-tests/gcovtester.py

While measuring the branch coverage we don't count JERRY_ASSERT s.

Branch coverage (covered branch/all branch):
sort function:
- before: 44/50
- after: 48/48

sort compare helper:
- before: 18/24
- after: 24/24

[repasics]

Closed PR because of moving into a unified PR.