[gjs/wip/ptomato/develop: 10/11] coverage: Misc refactors

[Philip Chimento <pchimento src gnome org>]

commit 9e5981cd5b72ed6693f2ace2f1302387b48295de
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Sep 23 22:02:46 2017 -0700

    coverage: Misc refactors
    
    - Enable branch coverage by default, and include default code coverage
      options in makefile so that branch coverage works
    
    - Eliminate some double exceptions being thrown in cache code
    
    - Fix unused parameter in C++ function
    
    - Remove unused constant dealing with binary cache which we don't have
      anymore
    
    - Tighten assertions in some tests
    
    - Remove or change some obsolete comments
    
    https://bugzilla.gnome.org/show_bug.cgi?id=788166

 Makefile-test.am                   |    2 +
 gjs/coverage.cpp                   |   57 ++++++++---------------------------
 installed-tests/js/testCoverage.js |   12 ++++++-
 modules/_bootstrap/coverage.js     |    2 +-
 test/gjs-test-coverage.cpp         |    1 +
 5 files changed, 27 insertions(+), 47 deletions(-)
---
diff --git a/Makefile-test.am b/Makefile-test.am
index 879363c..ce2cf0c 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -336,7 +336,9 @@ JS_LOG_DRIVER = $(LOG_DRIVER)
 JS_LOG_COMPILER = $$LOG_COMPILER $$LOG_FLAGS $(top_builddir)/minijasmine
 
 CODE_COVERAGE_IGNORE_PATTERN = */{include,mfbt,gjs/test,gjs/installed-tests}/*
+CODE_COVERAGE_BRANCH_COVERAGE = 1
 CODE_COVERAGE_GENHTML_OPTIONS =                        \
+       $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)        \
        lcov/coverage.lcov                              \
        --prefix $(abs_top_builddir)/lcov/org/gnome/gjs \
        --prefix $(abs_top_builddir)                    \
diff --git a/gjs/coverage.cpp b/gjs/coverage.cpp
index 21c1c7f..d7b6d49 100644
--- a/gjs/coverage.cpp
+++ b/gjs/coverage.cpp
@@ -431,21 +431,17 @@ get_array_from_js_value(JSContext             *context,
     if (element_clear_func)
         g_array_set_clear_func(c_side_array, element_clear_func);
 
-    if (JS_GetArrayLength(context, js_array, &js_array_len)) {
-        uint32_t i = 0;
-        JS::RootedValue element(context);
-        for (; i < js_array_len; ++i) {
-            if (!JS_GetElement(context, js_array, i, &element)) {
-                g_array_unref(c_side_array);
-                gjs_throw(context, "Failed to get function names array element %d", i);
-                return false;
-            }
+    if (!JS_GetArrayLength(context, js_array, &js_array_len)) {
+        g_array_unref(c_side_array);
+        return false;
+    }
 
-            if (!(inserter(c_side_array, context, element))) {
-                g_array_unref(c_side_array);
-                gjs_throw(context, "Failed to convert array element %d", i);
-                return false;
-            }
+    JS::RootedValue element(context);
+    for (uint32_t i = 0; i < js_array_len; ++i) {
+        if (!JS_GetElement(context, js_array, i, &element) ||
+            !inserter(c_side_array, context, element)) {
+            g_array_unref(c_side_array);
+            return false;
         }
     }
 
@@ -526,12 +522,12 @@ get_hit_count_and_line_data(JSContext       *cx,
                             int32_t         *line)
 {
     JS::RootedId hit_count_name(cx, gjs_intern_string_to_id(cx, "hitCount"));
-    if (!gjs_object_require_property(cx, obj, "function element",
+    if (!gjs_object_require_property(cx, obj, description,
                                      hit_count_name, hit_count))
         return false;
 
     JS::RootedId line_number_name(cx, gjs_intern_string_to_id(cx, "line"));
-    return gjs_object_require_property(cx, obj, "function_element",
+    return gjs_object_require_property(cx, obj, description,
                                        line_number_name, line);
 }
 
@@ -971,33 +967,6 @@ gjs_get_file_checksum(GFile *file)
     return checksum;
 }
 
-/* The binary data for the cache has the following structure:
- *
- * {
- *     array [ tuple {
- *         string filename;
- *         string? checksum;
- *         tuple? {
- *             mtime_sec;
- *             mtime_usec;
- *         }
- *         array [
- *             int line;
- *         ] executable lines;
- *         array [ tuple {
- *             int branch_point;
- *             array [
- *                 int line;
- *             ] exits;
- *         } branch_info ] branches;
- *         array [ tuple {
- *             int line;
- *             string key;
- *         } function ] functions;
- *     } file ] files;
- */
-const char *COVERAGE_STATISTICS_CACHE_BINARY_DATA_TYPE = "a(sm(xx)msaia(iai)a(is))";
-
 GBytes *
 gjs_serialize_statistics(GjsCoverage *coverage)
 {
diff --git a/installed-tests/js/testCoverage.js b/installed-tests/js/testCoverage.js
index bd35dbe..2d5389d 100644
--- a/installed-tests/js/testCoverage.js
+++ b/installed-tests/js/testCoverage.js
@@ -247,8 +247,16 @@ describe('Coverage', function () {
         ]);
 
         let expectedFunctionCountersArray = [
-            jasmine.objectContaining({ name: '(anonymous):2@123', hitCount: 1 }),
-            jasmine.objectContaining({ name: 'name:1@456', hitCount: 0 }),
+            {
+                key: '(anonymous):2@123',
+                line: 2,
+                hitCount: 1,
+            },
+            {
+                key: 'name:1@456',
+                line: 1,
+                hitCount: 0,
+            },
         ];
 
         let convertedFunctionCounters = Coverage._convertFunctionCountersToArray(functionsMap);
diff --git a/modules/_bootstrap/coverage.js b/modules/_bootstrap/coverage.js
index b6b7770..cb6b7ab 100644
--- a/modules/_bootstrap/coverage.js
+++ b/modules/_bootstrap/coverage.js
@@ -394,7 +394,7 @@ function _branchesToBranchCounters(branches, nLines) {
  *      "key" : { line, hitCount }
  * }
  * @key: a unique key for the function whose coverage is being determined
- * line: The line at which execution first started on this function.
+ * @line: The line at which this function is defined.
  * @coverage: array returned from Debugger.Script.getOffsetsCoverage() or null
  * (https://developer.mozilla.org/en-US/docs/Tools/Debugger-API/Debugger.Script)
  */
diff --git a/test/gjs-test-coverage.cpp b/test/gjs-test-coverage.cpp
index e1a0b02..f898ca0 100644
--- a/test/gjs-test-coverage.cpp
+++ b/test/gjs-test-coverage.cpp
@@ -1843,6 +1843,7 @@ test_coverage_cache_invalidation(gpointer      fixture_data,
 
     const gsize expected_len = G_N_ELEMENTS(expected);
     const char *record = line_starting_with(coverage_data_contents, "SF:");
+    g_assert_nonnull(record);
     g_assert(check_coverage_data_for_source_file(expected, expected_len, record));
 
     g_free(script_output_path);