[gjs] coverage: Suppress warnings about executing odd lines by default
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs] coverage: Suppress warnings about executing odd lines by default
- Date: Fri, 6 Jan 2017 07:32:03 +0000 (UTC)
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]