fix(core): allow skipped tests when matching mutants (#2487)

Allow for a coverage per test result for a non-existing test. This can happen when you add a `this.skip` inside a `it`. In that case, `beforeEach` still executes and can add test coverage.

We now ignore simply ignore this and log a debug message.

Fixes #2485

e2e/test/mocha-javascript/stryker.conf.json

@@ -2,7 +2,7 @@
   "$schema": "../../node_modules/@stryker-mutator/core/schema/stryker-schema.json",
   "testRunner": "mocha",
   "concurrency": 2,
-  "coverageAnalysis": "off",
+  "coverageAnalysis": "perTest",
   "reporters": ["clear-text", "html", "event-recorder"],
   "plugins": [
     "@stryker-mutator/mocha-runner"

e2e/test/mocha-javascript/test/unit/Circle.spec.js

@@ -10,4 +10,9 @@ describe('Circle', function() {
 
     expect(circumference).to.be.equal(expectedCircumference);
   });
+
+  it('should skip this test', function() {
+    getCircumference(2);
+    this.skip();
+  });
 });

e2e/test/mocha-javascript/verify/verify.ts

@@ -14,9 +14,10 @@ describe('Verify stryker has ran correctly', () => {
       metrics: produceMetrics({
         killed: 26,
         mutationScore: 74.29,
-        mutationScoreBasedOnCoveredCode: 74.29,
-        survived: 9,
-        totalCovered: 35,
+        mutationScoreBasedOnCoveredCode: 96.3,
+        noCoverage: 8,
+        survived: 1,
+        totalCovered: 27,
         totalDetected: 26,
         totalMutants: 35,
         totalUndetected: 9,

packages/core/src/mutants/findMutantTestCoverage.ts

@@ -1,9 +1,11 @@
 import { CompleteDryRunResult, TestResult, CoveragePerTestId } from '@stryker-mutator/api/test_runner';
 import { Mutant } from '@stryker-mutator/api/core';
-import { tokens } from '@stryker-mutator/api/plugin';
+import { commonTokens, tokens } from '@stryker-mutator/api/plugin';
 
 import { MatchedMutant } from '@stryker-mutator/api/report';
 
+import { Logger } from '@stryker-mutator/api/logging';
+
 import { coreTokens } from '../di';
 import StrictReporter from '../reporters/StrictReporter';
 
@@ -14,13 +16,14 @@ export interface MutantTestCoverage {
   mutant: Mutant;
 }
 
-findMutantTestCoverage.inject = tokens(coreTokens.dryRunResult, coreTokens.mutants, coreTokens.reporter);
+findMutantTestCoverage.inject = tokens(coreTokens.dryRunResult, coreTokens.mutants, coreTokens.reporter, commonTokens.logger);
 export function findMutantTestCoverage(
   dryRunResult: CompleteDryRunResult,
   mutants: readonly Mutant[],
-  reporter: StrictReporter
+  reporter: StrictReporter,
+  logger: Logger
 ): MutantTestCoverage[] {
-  const mutantTestCoverage = mapToMutantTestCoverage(dryRunResult, mutants);
+  const mutantTestCoverage = mapToMutantTestCoverage(dryRunResult, mutants, logger);
   reporter.onAllMutantsMatchedWithTests(mutantTestCoverage.map(toMatchedMutant));
   return mutantTestCoverage;
 }
@@ -37,8 +40,8 @@ function toMatchedMutant({ mutant, testFilter, coveredByTests, estimatedNetTime
   };
 }
 
-function mapToMutantTestCoverage(dryRunResult: CompleteDryRunResult, mutants: readonly Mutant[]) {
-  const testsByMutantId = findTestsByMutant(dryRunResult.mutantCoverage?.perTest, dryRunResult.tests);
+function mapToMutantTestCoverage(dryRunResult: CompleteDryRunResult, mutants: readonly Mutant[], logger: Logger) {
+  const testsByMutantId = findTestsByMutant(dryRunResult.mutantCoverage?.perTest, dryRunResult.tests, logger);
   const timeSpentAllTests = calculateTotalTime(dryRunResult.tests);
 
   const mutantCoverage = mutants.map((mutant) => {
@@ -71,17 +74,18 @@ function mapToMutantTestCoverage(dryRunResult: CompleteDryRunResult, mutants: re
   return mutantCoverage;
 }
 
-function findTestsByMutant(coverageData: CoveragePerTestId | undefined, allTests: TestResult[]) {
+function findTestsByMutant(coveragePerTest: CoveragePerTestId | undefined, allTests: TestResult[], logger: Logger) {
   const testsByMutantId = new Map<number, Set<TestResult>>();
-  coverageData &&
-    Object.entries(coverageData).forEach(([testId, coverageData]) => {
+  coveragePerTest &&
+    Object.entries(coveragePerTest).forEach(([testId, mutantCoverage]) => {
       const test = allTests.find((test) => test.id === testId);
       if (!test) {
-        throw new Error(
-          `Found test with id "${testId}" in coverage data, but not in the test results of the dry run. This shouldn't happen! Please report the issue at the issue tracker of your stryker test runner`
+        logger.debug(
+          `Found test with id "${testId}" in coverage data, but not in the test results of the dry run. Not taking coverage data for this test into account`
         );
+        return;
       }
-      Object.entries(coverageData).forEach(([mutantIdAsString, count]) => {
+      Object.entries(mutantCoverage).forEach(([mutantIdAsString, count]) => {
         if (count) {
           const mutantId = parseInt(mutantIdAsString, 10);
           let tests = testsByMutantId.get(mutantId);

packages/core/test/unit/mutants/findMutantTestCoverage.spec.ts

@@ -224,5 +224,26 @@ describe(sut.name, () => {
       };
       expect(reporterMock.onAllMutantsMatchedWithTests).calledWithMatch([sinon.match(expectedFirstMatch), sinon.match(expectedSecondMatch)]);
     });
+
+    it('should allow for non-existing tests (#2485)', () => {
+      // Arrange
+      const mutant1 = factory.mutant({ id: 1 });
+      const mutant2 = factory.mutant({ id: 2 });
+      const mutants = [mutant1, mutant2];
+      const dryRunResult = factory.completeDryRunResult({
+        tests: [factory.successTestResult({ id: 'spec1', timeSpentMs: 20 })], // test result for spec2 is missing
+        mutantCoverage: { static: {}, perTest: { spec1: { 1: 1 }, spec2: { 1: 0, 2: 1 } } },
+      });
+
+      // Act
+      const actualMatches = act(dryRunResult, mutants);
+
+      // Assert
+      expect(actualMatches.find((mutant) => mutant.mutant.id === 1)?.testFilter).deep.eq(['spec1']);
+      expect(actualMatches.find((mutant) => mutant.mutant.id === 2)?.coveredByTests).deep.eq(false);
+      expect(testInjector.logger.debug).calledWith(
+        'Found test with id "spec2" in coverage data, but not in the test results of the dry run. Not taking coverage data for this test into account'
+      );
+    });
   });
 });