'hybrid mutants' can leak to the next run --ignoreStatic

[nicojs]

Summary

Testing hybrid mutants with --ignoreStatic doesn't work as expected with --ignoreStatic.

For an explanation of what "hybrid" mutants are, see
https://github.com/stryker-mutator/mutation-testing-elements/pull/1547/files

The assumption was that we would be able to test hybrid mutants as regular non-static mutants without the state leaking out. This assumption is incorrect. See the next example (grabbed from expressjs):

From test/res.format.js

var app1 = express();

app1.use(function(req, res, next){
  res.format({ /* redacted for readability */ });
});

app1.use(function(err, req, res, next){
/* redacted for readability */ 
})

var app2 = express();

app2.use(function(req, res, next){
/* redacted for readability */ 
});

app2.use(function(err, req, res, next){
/* redacted for readability */ 
})

var app3 = express();

app3.use(function(req, res, next){
/* redacted for readability */ 
});

var app4 = express();

app4.get('/', function(req, res, next){
/* redacted for readability */ 
});

app4.use(function(err, req, res, next){
/* redacted for readability */ 
})

var app5 = express();

app5.use(function (req, res, next) {
/* redacted for readability */ 
});

describe('res', function(){
  /* a lot of tests here, using `app1` through `app5` */ 
});

As you can see, the tests themselves add static coverage in this example (the initialization of app1 through app5 is done in static context (no test was active yet)). Now imagine a test runner first testing hybrid mutants in scope and next running another non-static mutant. The result will be that these app1 through app5 variables will have loaded the hybrid mutants. The only way to clear them would be by restarting the test runner process.

The result is tons of runtime errors. See this report.

express-v6-concurrency-15-ignore-static.zip

I've rerun with this change (ignoring hybrid mutants):

diff --git a/packages/core/src/mutants/mutant-test-planner.ts b/packages/core/src/mutants/mutant-test-planner.ts
index 4bc9e051..dcc1a7c0 100644
--- a/packages/core/src/mutants/mutant-test-planner.ts
+++ b/packages/core/src/mutants/mutant-test-planner.ts
@@ -68,7 +68,7 @@ export class MutantTestPlanner {
       // If there was coverage information (coverageAnalysis not "off")
       const tests = this.testsByMutantId.get(mutant.id) ?? [];
       const coveredBy = toTestIds(tests);
-      if (!isStatic || (this.options.ignoreStatic && coveredBy.length)) {
+      if (!isStatic) {
         // If not static, or it was "hybrid" (both static and perTest coverage) and ignoreStatic is on.
         // Only run covered tests
         const netTime = calculateTotalTime(tests);

And the result seems to be a lot more consistent (about as consistent as StrykerJS v5). But entire (hybrid) functions are ignored now. Not sure that is what we want. See this report:

express-v6-concurrency-15-ignore-static-hybrid.zip

Note: running without --ignoreStatic works as expected.

I can think of 4 possible solutions here:

1. Fix
   Keep testing hybrid mutants, but make sure that the mutant isn't active yet during the loading of the files. Our test runners will have to know if it is running under --ignoreStatic. since you don't want that behavior when not running with --ignoreStatic. Both mocha and Jasmine support this. note: I've implemented this fix. See 'hybrid mutants' can leak to the next run --ignoreStatic #3442 (comment)
2. Known issue
   Keep behavior like it is now and try to educate people to initialize their suts in beforeEach. This will make it impossible to make --ignoreStatic the default because this is essentially a bug.
3. Ignore
   Always ignore hybrid mutants under --ignoreStatic
4. Configure
   Allow users to configure the behavior: --static run|ignore|hybrid

Stryker config

Using the epic/esm branch.

{
  "$schema": "../../../packages/core/schema/stryker-schema.json",
  "testRunner": "mocha",
  "coverageAnalysis": "perTest",
  "mochaOptions": {
    "require": ["test/support/env"],
    "files": ["test/*.js", "test/acceptance/*.js"]
  },
  "ignoreStatic": true
}

Test runner config

none

Stryker environment

Using the epic/esm branch.

+-- mocha@9.2.0

Test runner environment

mocha --require test/support/env --reporter spec --bail --check-leaks test/ test/acceptance/

Add stryker.log

stryker.log

[nicojs]

I thought about it this weekend and I think we don't really have a choice in the matter. It should be fixed. I went ahead and implemented the fix in #3443.

@simondel can you review this tomorrow?

[nicojs]

Closed with #3443