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

Running Stryker 4.0 beta on prettier gives errors in initial test run #2401

Closed
nicojs opened this issue Aug 19, 2020 · 6 comments · Fixed by #2518
Closed

Running Stryker 4.0 beta on prettier gives errors in initial test run #2401

nicojs opened this issue Aug 19, 2020 · 6 comments · Fixed by #2518
Assignees
Labels
🐛 Bug Something isn't working
Milestone

Comments

@nicojs
Copy link
Member

nicojs commented Aug 19, 2020

Originally from @brodybits 🎉

For Prettier, the initial test run went much faster when switching from Jest to the command runner. But for some reason I am getting many test failures from the initial test run.

We should investigate this.

@nicojs nicojs added the 🐛 Bug Something isn't working label Aug 19, 2020
@nicojs nicojs added this to the 4.0 milestone Aug 19, 2020
@nicojs
Copy link
Member Author

nicojs commented Aug 19, 2020

@brodybits, if you don't have time to investigate this, I'd be happy to take a look. I've created this issue so we don't lose track of it (I've marked this issue with the 4.0 milestone)

@brodybits
Copy link

brodybits commented Aug 19, 2020

updated September 2020:

I tried again with 4.0.0-beta.3 and 4.0.0-beta.8 here: brodybits/prettier#42 (maybe better to just start over).

Here are the issues I am hitting at this point:

  • some memory issues on a 32GB cloud instance, used node --max-old-space-size=40000 ./node_modules/.bin/stryker run as a workaround
  • I had to exclude some sources:
    • src/language-js/index.js - I encountered a type error similar to String literal mutator should not mutate DeclareModule node types #2399 with 4.0.0-beta.3 (I gave up more quickly with 4.0.0-beta.8)
    • src/language-js/needs-parens.js - memory issues after waiting with 4.0.0-beta.3 (I gave up on 4.0.0-beta.8), even with extremely high --max-old-space-size
    • src/language-js/printer-estree.js - I gave up after waiting 45 minutes for Stryker to mutate only this file (I gave up more quickly with 4.0.0-beta.8)
  • now resolved with 4.0.0-beta.8:
    • syntax errors on initial test run
    • a TSConfigPreprocessor step right after the Instrumenter took over 30 minutes
  • newly discovered with 4.0.0-beta.8:
    • Initial test run fails with a timeout, seems to take just over the hard-coded 5 minutes to finish the initial test run. I was able to make a hack to Stryker in my own workarea with 3x timeout to make it through the initial test run.

Thanks to @nicojs for breaking this and other issues out from #1514. Unfortunately I have to juggle multiple priorities at this point. It would really help if you want to take this on:)


P.S. I did try with Jest runner on 4.0.0-beta.3, encountered memory issues during initial test run on cloud instance with 32 GB. RAM, even with this command:

node --max-old-space-size=400000000 ./node_modules/.bin/stryker run --fileLogLevel trace --logLevel debug

@nicojs
Copy link
Member Author

nicojs commented Sep 25, 2020

I've taken a look at this one. Most of the issues can be blamed on the ConditionalExpression mutator. It also mutates SwitchCase statements.

This:

switch(foo){
  case 'bar':
    console.log('bar');
    break;
  case 'baz':
    console.log('baz');
    break;
}

Is instrumented as:

if (stryMutAct_9fa48(3)) {
  switch (foo) {
    case stryMutAct_9fa48(1) ? \\"\\" : (stryCov_9fa48(1), 'bar'):
      console.log(stryMutAct_9fa48(2) ? \\"\\" : (stryCov_9fa48(2), 'bar'));
      break;

    case 'baz':
  }
} else if (stryMutAct_9fa48(0)) {
  switch (foo) {
    case 'bar':
    case stryMutAct_9fa48(4) ? \\"\\" : (stryCov_9fa48(4), 'baz'):
      console.log(stryMutAct_9fa48(5) ? \\"\\" : (stryCov_9fa48(5), 'baz'));
      break;
  }
} else {
  stryCov_9fa48(0, 3);

  switch (foo) {
    case stryMutAct_9fa48(1) ? \\"\\" : (stryCov_9fa48(1), 'bar'):
      console.log(stryMutAct_9fa48(2) ? \\"\\" : (stryCov_9fa48(2), 'bar'));
      break;

    case stryMutAct_9fa48(4) ? \\"\\" : (stryCov_9fa48(4), 'baz'):
      console.log(stryMutAct_9fa48(5) ? \\"\\" : (stryCov_9fa48(5), 'baz'));
      break;
  }
}

Which is correct, but not very efficient. It duplicates the switch statement for each mutant. Prettier has very large switch-case expressions. For example, printer-estree has a switch expression with 303 switch-case expressions! So it creates 303 * 303 copies, just for that mutator alone.

We should find a different way of placing these mutants.

a TSConfigPreprocessor step right after the Instrumenter took over 30 minutes

I can't reproduce this. Preprocessors have had a serious change since the first beta releases, so maybe we fixed this? 🤷‍♂️

syntax errors on initial test run

I feel pretty confident that these have been fixed since you first tried this. I can't reproduce it with a subset of the files in prettier.

@brodybits
Copy link

Most of the issues can be blamed on the ConditionalExpression mutator.

Yup. Using recent 4.0.0-beta.8 seemed to resolve the syntax issues.

It also mutates SwitchCase statements.
[...]

Right. It would be nice if we can improve this, maybe in a separate issue?

Preprocessors have had a serious change since the first beta releases, so maybe we fixed this? 🤷‍♂️

Yes, I think this part is OK now.

syntax errors on initial test run

I feel pretty confident that these have been fixed

Yes.

I am now getting timeouts during the mutation testing, would appreciate any pointers.

@nicojs
Copy link
Member Author

nicojs commented Sep 28, 2020

It also mutates SwitchCase statements.
[...]
Right. It would be nice if we can improve this, maybe in a separate issue?

Let's do that on this issue.

I am now getting timeouts during the mutation testing, would appreciate any pointers.

Where exactly? In the dry run? Or during mutation testing?

I personally think prettier isn't the best repository to use Stryker on. Running the tests already takes several minutes. I don't think mutation testing in its current form is useful here. However, as a "stress test" for Stryker it is very valuable.

I'll remove the 4.0 milestone. Most issues are solved, the rest can be picked up after 4.0 release IMO

@brodybits
Copy link

PR #2518 seems to resolve the issue with src/language-js/printer-estree.js but not with 2 other modules:

src/language-js/index.js - results in this failure:

00:46:08 (43366) ERROR Stryker SyntaxError: src/language-js/index.js:87:6 expressionMutantPlacer could not place mutants with type(s): "StringLiteral". Either remove this file from the list of files to be mutated, or ignore the mutators. Please report this issue at https://github.com/stryker-mutator/stryker/issues/new?assignees=&labels=%F0%9F%90%9B+Bug&template=bug_report.md&title=expressionMutantPlacer%20could%20not%20place%20mutants%20with%20type(s)%3A%20%22StringLiteral%22.
  85 |     return require("./parser-babel").parsers.babel;
  86 |   },
> 87 |   get "babel-flow"() {
     |       ^^^^^^^^^^^^
  88 |     return require("./parser-babel").parsers["babel-flow"];
  89 |   },
  90 |   get "babel-ts"() {
    at File.buildCodeFrameError (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/core/lib/transformation/file/file.js:248:12)
    at NodePath.buildCodeFrameError (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/path/index.js:144:21)
    at Object.placeMutants (/home/brodybits/stryker/packages/instrumenter/dist/src/mutant-placers/index.js:31:28)
    at exit (/home/brodybits/stryker/packages/instrumenter/dist/src/transformers/babel-transformer.js:28:34)
    at NodePath._call (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/path/context.js:55:20)
    at NodePath.call (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/path/context.js:38:14)
    at NodePath.visit (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/path/context.js:101:8)
    at TraversalContext.visitQueue (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/context.js:112:16)
    at TraversalContext.visitSingle (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/context.js:84:19)
    at TraversalContext.visit (/home/brodybits/stryker/packages/instrumenter/node_modules/@babel/traverse/lib/context.js:140:19)

src/language-js/needs-parens.js - results in a memory issue after a bit of a delay

I am now able to see the mutation testing continue after the initial run when only mutating src/language-js/printer-estree.js.


I personally think prettier isn't the best repository to use Stryker on.

I would agree, and I find this to be very unfortunate. I had come to really like the way Stryker helped identify missing test on some other projects, and we now rely on it to check the test coverage of xmldom.

But I do remain hopeful that we can continue to make progress here, at least "as a stress test".

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
None yet
Development

Successfully merging a pull request may close this issue.

2 participants