stryker-mutator / stryker-js

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

Mutants surviving because it's trying to mutate logger.info #1470

Closed sswales closed 5 years ago

sswales commented 5 years ago

Summary

Mutants surviving because it's trying to mutate logger.info

When Stryker run's it is mutating the logger.info output which is for traceback, not actually affect the test.

Screenshot 2019-03-26 at 09 42 28 Screenshot 2019-03-26 at 09 44 19

Is there a way to make Stryker ignore these?

Stryker config

module.exports = function (config) {
  config.set({
    files: [
      'src/**/*',
      '!test/**/*',
    ],
    mutate: [
      'src/code1/**/*.js',
      'src/code2/**/*.js',
      '!src/common/**/*',
      '!src/**/*.test.js',
      '!src/**/*.json',
      '!src/**/*.ejs',
      '!src/**/__mocks__/*',
      '!test/**/*',
    ],
    timeoutMs: 10000,
    mutator: 'javascript',
    packageManager: 'npm',
    reporters: ['html', 'clear-text', 'progress', 'dashboard'],
    testRunner: 'jest',
    transpilers: [],
    coverageAnalysis: 'off',
    jest: { config: require('./.jestrc.unit.json'), enableFindRelatedTests: true },
    htmlReporter: { baseDir: 'reports/mutation' },
    thresholds: { high: 80, low: 50, break: 40 },
    allowConsoleColors: true,
  });
};

Stryker environment

+-- @stryker-mutator/core@1.1.1
+-- @stryker-mutator/api@1.1.1
+-- @stryker-mutator/util@1.1.1
+-- @stryker-mutator/html-reporter@1.1.
+-- @stryker-mutator/javascript-mutator@1.1.1
+-- @stryker-mutator/jest-runner@1.1.1
+-- jest@24.5.0

Test runner environment

# Test command
npm run stryker
{
  "collectCoverage": true,
  "collectCoverageFrom": [
    "src/**/*.test.{js,jsx}",
    "!<rootDir>/node_modules/",
    "!src/**/*.local.{js,jsx}"
  ],
  "coverageDirectory": "coverage",
  "coverageReporters": [
    "text"
  ],
  "roots": [
    "src/"
  ],
  "verbose": true
}

Add stryker.log

nicojs commented 5 years ago

No, currently there is no way to exclude specific mutations. However, it is possible to ignore entire files or mutators.

Please comment/upvote on one of these issues if you want to see a feature like this implemented: See #1464 and #1174.

Or comment on this issue if you want to see it implemented in a different way.

We probably won't implement this ourselves, however, we would be willing to accept PRs.

sswales commented 5 years ago

Thank you for the reply @nicojs

However, if we ignore the whole file because of logging probably means that we are not testing 80% of our tests, unfortunately. Also getting rid of specific mutators won't actually have any impact either.

It's highly likely that any company using AWS for applications will be using logger, so it's more than likely this will become a common issue as logging is needed.

Do you have any other ideas?

sswales commented 5 years ago

If would be great if in the config you could add a list of things to ignore for example:

    ignore: [
      'logger.info',
      'logger.warn'
    ],
nicojs commented 5 years ago

I think something like #1464 would work for you as well.

    ignore: [
      'logger.info',
      'logger.warn'
    ],

Would that mean you ignore all mutations for statements matching that pattern? Or only string mutations? How would the matching work? Just on text (string.startsWith)?

A bit more background:

I personally am against ignoring specific mutants. You're basically saying "we killed 100% of the mutants, except for the ones we didn't choose to kill".

Ignoring specific mutants is valid in some use cases, but it should be used with caution. This is why ignoring specific mutants is fairly low on our priority list. It might take a while before we're implementing it. We're happy to discuss and accept PR's for this.

sswales commented 5 years ago

Yes, I was meaning to ignore all mutations for statements matching that pattern.

So yes the approach of line.startsWith would be the easiest way to go...

In my specific example, we know there will be in every file at least a hand full of logger.info and it's no use to be tested, so it makes sense to have the ability to ignore this.

I understand your thoughts on people using it in a negative way, however, that comes down to the software engineer doing it their job correctly. And as by default, you will have this empty anyway you are not encouraging this to be used from the start.

The problem with just accepting a lower score is that if you have these logger events (in this case) skewing the test results. So you will have to manually check every run as you can't be confident what is causing this to be that low, so you need to check to ensure that the survived tests are only around the logger and not your actual code.

If this functionality existed and you could exclude logger, you know if it's not 100% something needs attention and you can be more productive with when you need to analyse the results.

nicojs commented 5 years ago

@shaunswales we're closing this issue in favor of one fresh issue, explaining our reasoning in a central place. Feel free to add your comments there. #1472

nicojs commented 10 months ago

Ignore-plugins are released in StrykerJS 7.3 🎉 . Ignoring something like logger.info should be pretty easy, if you want help please don't hesitate to ask.

https://twitter.com/stryker_mutator/status/1713557660832792870

https://stryker-mutator.io/docs/stryker-js/disable-mutants/#using-an-ignore-plugin