Multiple config workarounds needed for dry run to work for React [Stryker 4.0.0-beta]

[brodycj]

updated:

Question

As I said in #1514, I needed some config workarounds for the dry run to work with https://github.com/facebook/react:

• Group 1:
  • custom buildCommand setting
  • custom symlinkNodeModules setting (recommended)
• Group 2:
  • custom sandbox options

I got the dry run working on 4.0.0-beta.3 with the following config (with a bit trial-and-error):

/**
 * @type {import('@stryker-mutator/api/core').StrykerOptions}
 */
module.exports = {
  mutator: 'javascript',
  packageManager: 'yarn',
  reporters: ['html', 'clear-text', 'progress'],
  testRunner: 'command',
  transpilers: [],
  coverageAnalysis: 'off',
  // workarounds starting here:
  buildCommand: 'yarn',
  symlinkNodeModules: false,
  sandbox: {
    fileHeaders: {},
    stripComments: false
  },
};

I don't know if any of the config items may be redundant.

This is not a blocker but seems to be a bit counterintuitive.

Stryker environment

$ npm ls | grep stryker
├─┬ @stryker-mutator/[email protected]
│ ├─┬ @stryker-mutator/[email protected]
│ ├─┬ @stryker-mutator/[email protected]
│ │ ├── @stryker-mutator/[email protected] deduped
│ │ ├── @stryker-mutator/[email protected] deduped
│ ├─┬ @stryker-mutator/[email protected]

Test runner is command.

As a side point, I suspect it should be possible to get the Jest runner working with React with the right kind of utility script rework.

Additional context

I am generally testing with stryker.conf.js which seems to be a bit more flexible than working with JSON.

I did have to comment a shebang out of scripts/jest/jest.js in React to get past issue #2398.

I think this is less important than other issues #2400, #2399, #2398.

[nicojs]

Thanks again for the help!

I think the problems is mostly with the sandbox settings.

sandbox: {
    fileHeaders: {},
    stripComments: false
  },

(btw, great you found those!)

Ideally we don't want these settings here. However, they are needed for scenario's where you use typescript as a compiler. Typescript compilation normally fails (by design) because mutation switching introduces a lot of type errors (by design). To counteract this, we add the // @ts-nocheck on the top of each file. We also add eslint-disable just in case (for example, webpack compilation fails when you add the eslint plugin 🙄). This is what the default headers do:

sandbox: {
    fileHeaders: {
      "**/*+(.js|.ts|.cjs|.mjs)?(x)": "/* eslint-disable */\n// @ts-nocheck\n"
    }
}

The stripComments is another unfortunate addition we need in the typescript use case. If a user adds //@ts-expect-error somewhere, it will actually break the typescript compilation, since you expect an error while @ts-nocheck is active (no errors are reported). See microsoft/TypeScript#39427. Even if that is fixed, users might use @ts-check somewhere which turns typechecking back on. I was thinking that stripComments would be the best way to deal with this, since it is mostly safe to remove comments from code. I guess this isn't safe for jest projects, because of the @jest-environment comment, right? Ugh..

If someone can think of a better way to deal with compile errors, please let me know!

[brodycj]

updated:

I tried disabling my workaround options one-by-one. All are needed, including both sandbox options. Possible exception is symlinkNodeModules, but I would recommend keeping it to avoid a warning message.

I suspect that the buildCommand: 'yarn' override is needed because React is a Lerna monorepo.

---

I tried smarter sandbox options, with help from globster.xyz (linked from readme), and they seem to work for me with the React codebase sandbox issue went away due to an issue with comma-separated patterns

sandbox config, does not look right

  sandbox: {
    fileHeaders: {
      '**/!(.jest)+(.js|.ts|.cjs|.mjs)?(x),!**/__test__/**':
        '/* eslint-disable */\n// @ts-nocheck\n',
    },
    stripComments: '**/!(.jest)+(.js|.ts|.cjs|.mjs)?(x),!**/__test__/**',
  },

---

P.S. I did try using my local build of @stryker-mutator/core with some hacks to skip sources in the __tests__ subdirectory tree.

hacks to skip sources in __tests__

--- a/packages/core/src/sandbox/file-header-preprocessor.ts
+++ b/packages/core/src/sandbox/file-header-preprocessor.ts
@@ -17,4 +17,5 @@ export class FileHeaderPreprocessor implements FilePreprocessor {
   public async preprocess(files: File[]): Promise<File[]> {
     return files.map((file) => {
+      if (/__tests__/.test(file.name)) return file;
       Object.entries(this.options.sandbox.fileHeaders).forEach(([pattern, header]) => {
         if (minimatch(path.resolve(file.name), path.resolve(pattern))) {

--- a/packages/core/src/sandbox/strip-comments-preprocessor.ts
+++ b/packages/core/src/sandbox/strip-comments-preprocessor.ts
@@ -19,12 +19,13 @@ export class StripCommentsPreprocessor implements FilePreprocessor {
   public async preprocess(files: File[]): Promise<File[]> {
     if (this.options.sandbox.stripComments === false) {
       return files;
     } else {
       const pattern = path.resolve(this.options.sandbox.stripComments);
       return files.map((file) => {
+        if (/__tests__/.test(file.name)) return file;
         if (minimatch(path.resolve(file.name), pattern)) {
           return new File(file.name, stripComments(file.textContent));
         } else {
           return file;
         }
       });

I did verify during the dry run that with no custom sandbox options, Stryker modified the sources but not in the __tests__ subdirectory. But I still got test errors in the end. So there is something really strange going on with React.

---

I am now thinking we should just do the default fileHeaders and stripComments on TypeScript sources, as I proposed in PR #2406.

[nicojs]

I'll remove the milestone from this issue. I think we're pretty happy with the current status of mutation switching. We've removed the strange sandbox options. I think #2163 would also help with the symlinkNodeModules magic.

If you agree @brodybits than we can also close this issue.

[stale]

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.