stryker-mutator / stryker-js

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

Static mutants not being ignored #3774

Open gapark opened 1 year ago

gapark commented 1 year ago

Summary Static mutants still seem to be considered in the reporting despite running with the --ignoreStatic flag. This seems to potentially be limited to just the ArrowFunction mutants so far.

We are using Stryker with Jest in a NextJS project. We split our components up into an index.js and a helpers.js file. The ArrowFunction mutants that we're having issues with are always generated in these helper.js files, and stryker seems to fail to run any tests for these mutants so they survive everytime. Running without the --ignoreStatic flag causes these tests to run and kill these mutants. This is an issue for us as we enforce 100% of mutants killed in our CI pipelines.

image

I think this is possibly just an error with how reporting is being done for these mutants, as stryker seems to be ignoring them when running tests, but accounting for them in coverage reports.

Right now we are just using // Stryker disable next-line ArrowFunction where needed, but may move to disabling this mutant globally. This is obviously not ideal but we've been struggling to find another solution.

Stryker config This is the content of our stryker.conf.mjs file:

const config = {
  packageManager: 'yarn',
  reporters: [
    'clear-text',
    'html',
    'progress'
  ],
  testRunner: 'jest',
  jest: {
    projectType: 'custom',
    configFile: 'jest.config.js'
  },
  coverageAnalysis: 'perTest',
  ignorePatterns: ['/node_modules/', '/.next/'],
  logLevel: 'info',
  mutate: [
    'src/**/*.js',
    '!src/**/*styles.js',
    '!src/**/*constants.js',
    '!src/constants/*.js'
  ],
  timeoutMS: 15000
}

export default config

Test runner config This is the content of our jest.config.js file:

module.exports = {
  clearMocks: true,

  setupFilesAfterEnv: ['<rootDir>/test/setup.js'],

  testPathIgnorePatterns: ['/node_modules/', '/.next/'],

  transformIgnorePatterns: [
    'node_modules/(?!@internal)'
  ],

  collectCoverageFrom: [
    'src/**/*.js'
  ],

  coverageThreshold: {
    global: {
      branches: 100,
      functions: 100,
      lines: 100,
      statements: 100
    }
  },

  moduleNameMapper: {
    '\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': '<rootDir>/test/__mocks__/fileMock.js',
    '@components/(.*)': '<rootDir>/src/components/$1',
    '@constants/(.*)': '<rootDir>/src/constants/$1',
    '@helpers/(.*)': '<rootDir>/src/helpers/$1',
    '@hooks/(.*)': '<rootDir>/src/hooks/$1',
    '@root/(.*)': '<rootDir>/$1',
    '@scripts/(.*)': '<rootDir>/src/scripts/$1',
    '@src/(.*)': '<rootDir>/src/$1',
    '@tracker/(.*)': '<rootDir>/src/tracker/$1'
  },

  testEnvironment: 'jsdom',

  transform: {
    '^.+\\.js?$': '<rootDir>/node_modules/babel-jest'
  }
}

Stryker environment

├─ @stryker-mutator/api@6.1.2
├─ @stryker-mutator/core@6.1.2
├─ @stryker-mutator/instrumenter@6.1.2
├─ @stryker-mutator/jest-runner@6.1.2
└─ @stryker-mutator/util@6.1.2
├─ jest@29.1.2
├─ jest-environment-jsdon@29.1.2
├─ jest-next-dynamic@1.0.1
├─ babel-jest@29.1.2
├─ @testing-library/dom@8.11.1
├─ @testing-library/jest-dom@5.16.4
├─ @testing-library/react-hooks@8.0.1
├─ @testing-library/react@13.3.0
└─ @testing-library/user-event@14.4.3

Test runner environment

NODE_ENV=test jest ./test

Your Environment

software version(s)
node 16.17.1
npm N/A
yarn 1.22.19
Operating System Ubuntu 20.04

stryker.log stryker.log

nicojs commented 1 year ago

👋 Hi @gapark thanks for opening this issue

This is a hybrid mutant. So it simultaneously has static and runtime coverage. See https://stryker-mutator.io/docs/mutation-testing-elements/static-mutants/#hybrid-mutants

When you use --ignoreStatic, static mutants are ignored. For hybrid mutants, Stryker assumes that the mutant should be killed by the tests that cover the mutant at runtime and they are therefore handled as any other runtime mutant. Thus, in a sense, Stryker will ignore the static part of hybrid mutants.

gapark commented 1 year ago

Thanks for the response @nicojs

In this case do we expect it not to run tests for the static part of the mutant then? Should the static part still be counted for coverage if tests aren't being ran for it?

Take the example I posted originally: image

This function has three test cases in our codebase:

it('returns false if not an array', () => {
  expect(hasContexts()).toEqual(false)
})

it('returns false if empty array', () => {
  expect(hasContexts([])).toEqual(false)
})

it('returns false if not empty array', () => {
  expect(hasContexts([1, 2, 3])).toEqual(true)
})

The problem we are seeing is that these tests are not ran for these hybrid ArrowFunction mutants, meaning it is never killed despite the tests existing to kill it. The hasContexts function is also used inside of a react component. This component has its own tests which are ran for this mutant however. If stryker were to run the tests associated with this function, then the mutant would be killed. It runs these tests for other mutants as expected, just not these ArrowFunction ones that we can't seem to kill.

nicojs commented 1 year ago

Wow that sounds like a bug indeed.

  1. Can you verify that the tests are executed during dry run? (can you see them in the "🧪Tests" view?)
  2. What happens if you disable findRelatedTests in Stryker config?
gapark commented 1 year ago

@nicojs I'm so sorry for the incredible delay in response, I have been off work for a little while.

I can no longer replicate this on the branch I originally posted with. However, the problem is still occurring on other branches we've witnessed it on.

image

Both of these functions are having ArrowFunction mutants generated. In this instance, I can actually see the tests in the "Tests" tab of the report, but this was definitely not the case on the other branch. Tests below: image image

As far as I can tell, both these mutants should have been killed by their respective tests as they are asserting a value other than undefined was returned.

Unfortunately when running with findRelatedTests disabled, everthing times out. We have over 1500 tests in this project and running that many just seems to not be possible on my hardware.

nicojs commented 1 year ago

Could you try running jest on that file brand_offer_list/helpers.js file? Something like:

jest --findRelatedTests brand_offer_list/helpers.js

Does this work? Or does jest say it cannot find any tests?

jessephelps commented 1 year ago

We're experiencing what I believe is a related issue. When we run mutate on all matching files there are 3 static mutants that survive. If we specify on the CLI --mutate specific/file.tsx then Stryker recognizes those 3 static mutants are to be ignored and reports that all mutants are killed.

When I run it with --mutate specific/file.tsx

Screen Shot 2022-10-27 at 9 07 23 AM

When I run it without the specific file and just let it run over all matching .tsx files.

Screen Shot 2022-10-27 at 9 09 39 AM
gapark commented 1 year ago

@nicojs Running jest --findRelatedTests brand_offer_list/helpers.js seems to work. It finds all 10 related test suites.

@jessephelps This does seem somewhat similar. However, we are seeing the problem regardless of whether it is 1 file or all matching files that are being mutated.

nicojs commented 1 year ago

This seems to be a bug in Stryker 🐛. Maybe in the jest runner plugin, or in the Stryker core.

@gapark @jessephelps Could you provide me with a small reproduction repo? That would be awesome! Either on GitHub or with a zip file attached here would be great!

gapark commented 1 year ago

@nicojs I had a quick attempt at this but our project is quite a substantial one, so trying to pull out everything sensitive broke a lot of things. I'll try put some time aside during my next sprint to see if I can clean up the repo and keep everything working.

gapark commented 1 year ago

Just updating that I haven't forgotten about this. I'm still struggling to split out anything private from the repo without it breaking due to time constraints.

nicojs commented 1 year ago

I understand! It might be faster to start with an empty project and copy+paste stuff in until you've reproduced it.

gapark commented 1 year ago

That is actually a much better shout. I'll give that a go sometime in the next week, I'll block of some time in my calendar. 😄

gapark commented 1 year ago

Another update: still not been able to put the time aside for this.

Black Friday and the lead up to Christmas is a very busy period for us so I've just not had the capacity at all. Thankfully with the end of the month coming up things will slow down and most people will be away for the holidays, which should definitely allow me to focus on getting a sample repo for you @nicojs

Sorry about the delays, and thank you for your patience.

mkizka commented 1 year ago

I have the same issue. In my case, the survived file has static mutants called by await import(). This is a simple repository to reproduce.

$ git clone https://github.com/mkizka/stryker-js-static-mutants
$ cd stryker-js-static-mutants
$ yarn
$ yarn stryker run --ignoreStatic
// greet.ts
const hi = "👋";

export function greet(name) {
  return `${hi} ${name}`;
}

export const arrowGreet = (name) => `${hi} ${name}`;

// greet.test.ts
import { arrowGreet, greet } from "./greet";

it("should greet me", () => {
  expect(greet("me")).toBe("👋 me");
});

it("should greet me 2", () => {
  expect(arrowGreet("me")).toEqual("👋 me");
});
// dynamic.ts
export async function dynamic() {
  await import("./greet");
  return "dynamic function";
}

// dynamic.test.ts
import { dynamic } from "./dynamic";

it("dynamic", async () => {
  expect(await dynamic()).toBe("dynamic function");
});

image

Comment out await import or disabling ignoreStatic makes the score 100%. I understand that this behavior is hybrid mutants, but I expected mutants killed or ignored.

stryker.log

stale[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.