stryker-mutator / stryker-js

Mutation testing for JavaScript and friends
https://stryker-mutator.io
Apache License 2.0
2.55k stars 242 forks source link

False positive "Survived" in a mkdirSync #4739

Closed dominikzogg closed 3 months ago

dominikzogg commented 4 months ago

I see a false positive there. The first time the code get excuted the folder /tmp/multipart would not exist, on second run it does. Therefore it looks like removing { recursive: true } wouldn't make a difference, but it does.

chubbyts-http-multipart

Stryker config

{
  "$schema": "./node_modules/@stryker-mutator/core/schema/stryker-schema.json",
  "testRunner": "jest",
  "coverageAnalysis": "off",
  "reporters": [
    "clear-text",
    "progress",
    "dashboard"
  ]
}

Stryker environment

"@stryker-mutator/core": "^8.0.0",
"@stryker-mutator/jest-runner": "^8.0.0",
"jest": "^29.7.0",
"ts-jest": "^29.1.1",

Test runner environment

npm run infection // "infection": "stryker run"
{
  "jest": {
    "preset": "ts-jest",
    "testEnvironment": "node",
    "collectCoverageFrom": [
      "src/**/*.ts"
    ],
    "coverageThreshold": {
      "global": {
        "lines": 100
      }
    },
    "coveragePathIgnorePatterns": [
      "src/index.ts"
    ]
  }
}

Your Environment

software version(s)
node v20.11.1
npm 10.2.4
Operating System Linux

Add stryker.log

> stryker run --fileLogLevel trace

16:43:16 (20358) INFO ProjectReader Found 1 of 22 file(s) to be mutated.
16:43:16 (20358) INFO Instrumenter Instrumented 1 source file(s) with 30 mutant(s)
16:43:16 (20358) INFO ConcurrencyTokenProvider Creating 19 test runner process(es).
16:43:17 (20358) INFO DryRunExecutor Starting initial test run (jest test runner with "off" coverage analysis). This may take a while.
16:43:18 (20358) INFO DryRunExecutor Initial test run succeeded. Ran 3 tests in 1 second (net 15 ms, overhead 1164 ms).
Mutation testing  [==================================================] 100% (elapsed: <1m, remaining: n/a) 30/30 Mutants tested (2 survived, 2 timed out)

All tests/multipart-middleware.test.ts
  ✓ createMultipartMiddleware without content-type [line 55] (killed 6)
  ✓ createMultipartMiddleware without multipart/form-data [line 73] (killed 1)
  ✓ createMultipartMiddleware with multipart/form-data [line 91] (killed 19)

[Survived] BooleanLiteral
src/multipart-middleware.ts:24:45
-         mkdirSync(temporaryPath, { recursive: true });
+         mkdirSync(temporaryPath, { recursive: false });

[Survived] ObjectLiteral
src/multipart-middleware.ts:24:32
-         mkdirSync(temporaryPath, { recursive: true });
+         mkdirSync(temporaryPath, {});

Ran 2.80 tests per mutant on average.
-------------------------|---------|----------|-----------|------------|----------|----------|
File                     | % score | # killed | # timeout | # survived | # no cov | # errors |
-------------------------|---------|----------|-----------|------------|----------|----------|
All files                |   93.33 |       26 |         2 |          2 |        0 |        0 |
 multipart-middleware.ts |   93.33 |       26 |         2 |          2 |        0 |        0 |
-------------------------|---------|----------|-----------|------------|----------|----------|
16:43:26 (20358) INFO DashboardReporter The report was not send to the dashboard. The dashboard.project and/or dashboard.version values were missing and not detected to be running on a build server.
16:43:26 (20358) INFO MutationTestExecutor Done in 10 seconds.
nicojs commented 4 months ago

I'm trying to reproduce it, but regular unit tests fail on my machine. See the failing test output below. I've commented-out the failing assertion and was able to run the tests.

Next, I made the change as StrykerJS does. The tests still succeed. This leads me to conclude that it is a valid surviving mutant. You're missing a test to verify that recursive is correctly set.

diff --git a/src/multipart-middleware.ts b/src/multipart-middleware.ts
index 43c4c6f..c6e6a91 100644
--- a/src/multipart-middleware.ts
+++ b/src/multipart-middleware.ts
@@ -21,7 +21,7 @@ export const createMultipartMiddleware = (limits: busboy.Limits | undefined = un
     const body = await new Promise<PassThrough>((resolve) => {
       const temporaryPath = `${tmpdir()}/multipart/${randomBytes(64).toString('hex')}`;

-      mkdirSync(temporaryPath, { recursive: true });
+      mkdirSync(temporaryPath);

       const newBody = new PassThrough();
Iniital failing test output ``` FAIL tests/multipart-middleware.test.ts createMultipartMiddleware ✓ without content-type (6 ms) ✓ without multipart/form-data (1 ms) with multipart/form-data ✕ successful (12 ms) ✓ allow only 1 file (2 ms) ● createMultipartMiddleware › with multipart/form-data › successful expect(received).toBe(expected) // Object.is equality Expected: "e0ebc9b689335812e3b1a3abde4efb149f46c695" Received: "da39a3ee5e6b4b0d3255bfef95601890afd80709" 148 | ), 149 | ), > 150 | ).toBe(sha1(readFileSync(blueImagePath))); | ^ 151 | 152 | return response; 153 | }, at Object.callback (tests/multipart-middleware.test.ts:150:15) at Object. (tests/multipart-middleware.test.ts:159:14) Test Suites: 1 failed, 1 total ```
dominikzogg commented 4 months ago

@nicojs on the CI the test does not fail: https://github.com/chubbyts/chubbyts-http-multipart/actions/runs/8034292527/job/21945721235 if i remove the mkdirSync(temporaryPath, { recursive: true }); it fails. It seems your running in another issue.

nicojs commented 4 months ago

Ah, yes, I've taken another look. If you first run the tests once, then set { recursive: false } and rerun them, you'll see that they will pass. This is a 'bug' in the test suite: they don't reset shared state before each test.

Also, since all tests share the same state, they cannot run concurrently, see https://stryker-mutator.io/docs/stryker-js/parallel-workers/

I've created a PR to fix these issues. If you have any more questions, feel free to let me know.