Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stryker run succeeds when something goes wrong in the initial test run #1996

Closed
anthony-telljohann opened this issue Jan 28, 2020 · 19 comments
Closed
Labels
🐛 Bug Something isn't working
Projects
Milestone

Comments

@anthony-telljohann
Copy link

Scenario: The one where something went wrong in the initial test run

Given something went wrong in the initial test run
When running mutation tests with stryker run
Then ...

Personally, I would expect stryker run to fail but to my surprise it succeeds.

stryker.conf.js

module.exports = function(config) {
  config.set({
    logLevel: 'error',
    mutate: [
      '{src,lib}/**/*.ts?(x)',
      '!{src,lib}/**/?(*.)+(spec|fixture).ts?(x)',
    ],
    mutator: 'typescript',
    transpilers: ['typescript'],
    packageManager: 'npm',
    reporters: ['clear-text', 'progress', 'html'],
    testRunner: 'jest',
    jest: {
      enableFindRelatedTests: false,
      config: {
        preset: 'ts-jest',
        testEnvironment: 'node',
        resetModules: true,
      },
    },
    thresholds: { high: 100, low: 100, break: 100 },
    coverageAnalysis: 'off',
    tsconfigFile: 'tsconfig.json',
    htmlReporter: {
      baseDir: 'reports/coverage/mutation',
    },
    timeoutMS: 60000,
  })
}

jest.config.js

module.exports = {
  preset: 'ts-jest',
  testEnvironment: 'node',
  collectCoverageFrom: ['**/*.ts', '!**/fixtures/**'],
  coverageDirectory: 'reports/coverage/unit',
  coverageThreshold: {
    global: {
      branches: 100,
      functions: 100,
      lines: 100,
      statements: 100,
    },
  },
  reporters: [
    'default',
    [
      'jest-junit',
      {
        outputDirectory: '<rootDir>/reports/tests',
        outputName: 'unit-test-evidence.xml',
      },
    ],
    [
      'jest-html-reporter',
      {
        outputPath: '<rootDir>/reports/cucumber/requirements-evidence.html',
      },
    ],
  ],
}

Stryker environment

├── @stryker-mutator/core@2.5.0
├── @stryker-mutator/html-reporter@2.5.0
├── @stryker-mutator/jest-runner@2.5.0
├── @stryker-mutator/typescript@2.5.0
├── jest@24.9.0

Test command

stryker run

Environment

software version(s)
node v12.14.1
npm 6.13.6
Operating System macOS 10.14.6 (Mojave)

stryker.log

Starting task 'mutation test'
+ npm run test:mutation
> stryker run

INFO ConfigReader Using stryker.conf.js in the current working directory.
INFO TypescriptConfigEditor Loading tsconfig file /path/to/project/tsconfig.json
INFO BroadcastReporter Detected that current console does not support the "progress" reporter, downgrading to "progress-append-only" reporter
INFO InputFileResolver Found 9 of 37 file(s) to be mutated.
INFO InitialTestExecutor Starting initial test run. This may take a while.
ERROR StrykerCli an error occurred Error: Something went wrong in the initial test run
    at InitialTestExecutor.validateResult (/path/to/project/node_modules/@stryker-mutator/core/src/process/InitialTestExecutor.js:86:15)
    at InitialTestExecutor.run (/path/to/project/node_modules/@stryker-mutator/core/src/process/InitialTestExecutor.js:41:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)
INFO StrykerCli Trouble figuring out what went wrong? Try `npx stryker run --fileLogLevel trace --logLevel debug` to get some more info.

Finished task 'mutation test' with result: Success
@anthony-telljohann anthony-telljohann added the 🐛 Bug Something isn't working label Jan 28, 2020
@OriginalError
Copy link

Yeah, it would be nice to fail out with something along the lines of "Mutation Coverage Testing requires a green suite" an an exit code of NOT SUCCESS.

@jerfos25
Copy link

Agreed. It is not helpful to have a passing message with error indicators. This is misleading.

@jmtelljohann
Copy link

I run into this issue a lot. Stryker will throw an error during the initial run and my build will pass because it exits successfully. This means you have to look at the logs to know it failed.

@blastedt
Copy link

blastedt commented Jan 28, 2020

Looks like the Stryker CLI tries to kill its own process with 1, but something is ignoring that: https://github.com/stryker-mutator/stryker/blob/master/packages/core/src/StrykerCli.ts#L155-L160

@blastedt
Copy link

I believe this can be fixed by replacing lines 159 and 160 of the StrykerCli ts file with process.exit(1)

@anthony-telljohann
Copy link
Author

What are your thoughts @nicojs ?

@simondel
Copy link
Member

Simply killing the process sounds like a good plan. Would one of you be willing to add this?

I’m actually a bit surprised that this issue is found by so many people so suddenly 🤨 since it’s been in the code for quite a while I think

@bartekleon
Copy link
Member

@simondel It might be some other change that just let this issue happen.

@blastedt
Copy link

blastedt commented Feb 1, 2020

Willing to add this but my employment requires approval before I can contribute to open source projects. I'll open a PR once that gets through (mid next week?) unless one of y'all adds it first.

@jmtelljohann
Copy link

@simondel I've actually been experiencing this issue for a while, but kept forgetting to report it until a couple days ago.

@nicojs
Copy link
Member

nicojs commented Feb 2, 2020

Great find! Thanks for all the reactions here.

Please don't kill the process with process.exit. Instead use process.exitCode = 1. Simply killing the current process is not nice for use cases where Stryker is run as a plugin of something else. We've found this quite frustrating ourselves if test runners we support simply exit all of a sudden.

It's also described in the nodejs docs: https://nodejs.org/api/process.html#process_process_exit_code

In most situations, it is not actually necessary to call process.exit() explicitly. The Node.js process will exit on its own if there is no additional work pending in the event loop. The process.exitCode property can be set to tell the process which exit code to use when the process exits gracefully.

@anthony-telljohann
Copy link
Author

...surprised that this issue is found by so many people so suddenly
@simondel

We all experienced the same issue on our own projects throughout the last year. Recently, I made a change to all of our projects and it happened to everyone. Once I submitted the issue, I shared it with every team that may be interested in supporting the fix.

@simondel
Copy link
Member

simondel commented Feb 2, 2020

@nicojs You're right, I forgot that was the reason we do it that way! We should add that as a comment.

@anthony-telljohann Since your teams are using the same process, I assume they're also using similar hardware (i.e. macOS). Could you try to reproduce your problem using this demo-app?

Reproduction:

git clone https://github.com/stryker-mutator/robobar-example.git -b exit-code
cd ./robobar-example
npm i
npm run stryker

This will run mutation testing, which will fail and then Stryker should exit. If the demo-app reproduces your problem (i.e. it keeps on going), it should open a browser with the app which should look like: https://stryker-mutator.io/robobar-example/#!/

@anthony-telljohann
Copy link
Author

anthony-telljohann commented Feb 5, 2020

We certainly can and will do that 👍. That being said, we've replicated this issue on macOS, Windows, and Linux.

@simondel simondel added this to the 3.0 milestone Feb 12, 2020
@simondel simondel added this to Planning in Backlog Feb 12, 2020
@nicojs nicojs moved this from Planning to In Progress in Backlog Feb 14, 2020
@nicojs
Copy link
Member

nicojs commented Feb 14, 2020

@simondel are you the mole? 🙈

Your exit-code branch on the robobar example actually always executes both commands, regardless of exit code. This is because you used & to separate the commands instead of && 😅

I found the issue:

https://github.com/stryker-mutator/stryker/blob/fbb8102dc890521a80ef1f87236193e506ab9922/packages/core/src/StrykerCli.ts#L160

Don't know why we're using process.kill, since that overrides the exit code (which we've just set). removing it fixes the issue.

@nicojs
Copy link
Member

nicojs commented Feb 14, 2020

I've pushed the fix to master by accident 🙈

(I've restricted the master branch to prevent this from now on)

Anyway, fixed with 49c5162

@nicojs nicojs closed this as completed Feb 14, 2020
Backlog automation moved this from In Progress to Done Feb 14, 2020
@blastedt
Copy link

Thank you Nico! I am sorry I wasn't able to find the time to fix this myself. We appreciate the help.

@anthony-telljohann
Copy link
Author

@nicojs @simondel Is there any chance we could get a patch version released that includes these changes?

@simondel
Copy link
Member

Hi! This has been released with @stryker-mutator version 3! You can read all about it here: https://stryker-mutator.io/blog/2020-03-11/stryker-version-3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working
Projects
Development

No branches or pull requests

8 participants