[gjs] coverage: Suppress warnings about executing odd lines by default



commit 3f4294fc2c66971fcfb17e82cad8c1200daf6a7a
Author: Sam Spilsbury <smspillaz gmail com>
Date:   Mon Jun 15 14:59:59 2015 +0800

    coverage: Suppress warnings about executing odd lines by default
    
    Where the JS engine actually starts executing isn't entirely
    deterministic - its possible to make a reasonable guess as to
    what is executable, which is why the Reflect machinery exists,
    but we could always end up executing a weird line. The default
    behaviour has always been underestimate what lines are executable
    as opposed to overestimating, such that the coverage tools are
    not useful.
    
    There are a few not-well-understood cases where we can end
    up executing a line which we didn't think was executable based
    on analysis of the source code and the warnings generated
    can be quite noisy. Unfortunately, they can also interfere
    with tests that need to check the stderr for messages.
    
    Testing over the last year has shown that we've picked up the
    vast majority of cases where lines were marked non-executable
    when they actually were executable. As such, the utility
    of these warnings no longer outweighs their cost.
    
    The default behaviour is now to suppress these messages by
    default and provide an environment variable,
    GJS_DEBUG_EXECUTED_LINES to show the warnings about executing
    weird lines once again.
    
    Original patch by Sam Spilsbury, fixed up and rebased on current GJS by
    Philip Chimento.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751146

 gjs/coverage.cpp                   |    4 +++-
 installed-tests/js/testCoverage.js |    3 ++-
 modules/coverage.js                |   13 ++++++++-----
 3 files changed, 13 insertions(+), 7 deletions(-)
---
diff --git a/gjs/coverage.cpp b/gjs/coverage.cpp
index 2355b08..2ab8b48 100644
--- a/gjs/coverage.cpp
+++ b/gjs/coverage.cpp
@@ -1672,9 +1672,11 @@ bootstrap_coverage(GjsCoverage *coverage)
         /* Now create the array to pass the desired prefixes over */
         JSObject *prefixes = gjs_build_string_array(context, -1, priv->prefixes);
 
-        JS::AutoValueArray<2> coverage_statistics_constructor_args(context);
+        JS::AutoValueArray<3> coverage_statistics_constructor_args(context);
         coverage_statistics_constructor_args[0].setObject(*prefixes);
         coverage_statistics_constructor_args[1].set(cache_value);
+        coverage_statistics_constructor_args[2]
+            .setBoolean(g_getenv("GJS_DEBUG_COVERAGE_EXECUTED_LINES"));
 
         JSObject *coverage_statistics = JS_New(context,
                                                coverage_statistics_constructor,
diff --git a/installed-tests/js/testCoverage.js b/installed-tests/js/testCoverage.js
index 8224d93..6d2b926 100644
--- a/installed-tests/js/testCoverage.js
+++ b/installed-tests/js/testCoverage.js
@@ -1023,7 +1023,8 @@ describe('Coverage.incrementFunctionCounters', function () {
             undefined
         ];
 
-        Coverage._incrementExpressionCounters(expressionCounters, 'script', 2);
+        Coverage._incrementExpressionCounters(expressionCounters, 'script', 2,
+            true);
 
         expect(window.log).toHaveBeenCalledWith("script:2 Executed line " +
             "previously marked non-executable by Reflect");
diff --git a/modules/coverage.js b/modules/coverage.js
index 9f4141b..636ce1a 100644
--- a/modules/coverage.js
+++ b/modules/coverage.js
@@ -618,10 +618,12 @@ function _incrementFunctionCounters(functionCounters,
  * expressonCounters: An array of either a hit count for a found
  * executable line or undefined for a known non-executable line.
  * line: an executed line
+ * @shouldWarn: if true, print a mostly harmless warning about executing a line
+ * that was thought non-executable.
  */
 function _incrementExpressionCounters(expressionCounters,
                                       script,
-                                      offsetLine) {
+                                      offsetLine, shouldWarn) {
     let expressionCountersLen = expressionCounters.length;
 
     if (offsetLine >= expressionCountersLen)
@@ -646,8 +648,9 @@ function _incrementExpressionCounters(expressionCounters,
      * and BlockStatement, neither of which would ordinarily be
      * executed */
     if (expressionCounters[offsetLine] === undefined) {
-        log(script + ':' + offsetLine + ' Executed line previously marked ' +
-            'non-executable by Reflect');
+        if (shouldWarn)
+            log(script + ':' + offsetLine + ' Executed line previously marked ' +
+                'non-executable by Reflect');
         expressionCounters[offsetLine] = 0;
     }
 
@@ -868,7 +871,7 @@ function CoverageStatisticsContainer(prefixes, cache) {
  *
  * It isn't poissible to unit test this class because it depends on running
  * Debugger which in turn depends on objects injected in from another compartment */
-function CoverageStatistics(prefixes, cache) {
+function CoverageStatistics(prefixes, cache, shouldWarn) {
     this.container = new CoverageStatisticsContainer(prefixes, cache);
     let fetchStatistics = this.container.fetchStatistics.bind(this.container);
     let deleteStatistics = this.container.deleteStatistics.bind(this.container);
@@ -954,7 +957,7 @@ function CoverageStatistics(prefixes, cache) {
             try {
                 _incrementExpressionCounters(statistics.expressionCounters,
                                              frame.script.url,
-                                             offsetLine);
+                                             offsetLine, shouldWarn);
                 this._branchTracker.incrementBranchCounters(offsetLine);
             } catch (e) {
                 /* Something bad happened. Log the exception and delete


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]