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

perf: reduce next stryker runs #1610

Closed
bartekleon opened this issue Jun 27, 2019 · 10 comments
Closed

perf: reduce next stryker runs #1610

bartekleon opened this issue Jun 27, 2019 · 10 comments
Labels
🚀 Feature request New feature request

Comments

@bartekleon
Copy link
Member

bartekleon commented Jun 27, 2019

I have been lurking through issues and files, and I have found, that we could improve stryker performance, by checking, which files have been updated since the last commit.

The easiest way would be:
git diff HEAD^ HEAD --name-only

we could use this command in JS simply with exec or spawn:

const child = require('child_process');

child.spawnSync('git', ['diff', 'HEAD^', 'HEAD' ,'--name-only']).stdout.toString()

git diff HEAD^ --name-only with staged change
image

git diff HEAD^ HEAD --name-only with staged change
image

git diff HEAD^ --name-only without staged change
image

I guess it can be customizable for user, for example, to choose between git diff HEAD^ HEAD --name-only and git diff HEAD^ --name-only or to switch it off.

@bartekleon
Copy link
Member Author

bartekleon commented Jul 2, 2019

After small rethinking, options would be enableGitOptimisation: boolean | string = false
if true - by default it would be HEAD. string is just an initial commit from which it should watch files and stryker will check all files between that commit and what is now git diff string --name-only command.
Basically, we can just do a check for that option while getting files, then just compare if files from git command and from files: [] are the same:

if(enableGitOptimisation !== false) {
   let filesFromGit;
   if(enableGitOptimisation === true) {
      filesFromGit = child_process.execSync('git', ['diff', 'HEAD', '--name-only']).stdout.toString().split('\n').map(el => el.trim());
   } else {
      filesFromGit = child_process.execSync('git', ['diff', enableGitOptimisation, '--name-only']).stdout.toString().split('\n').map(el => el.trim());
  }
   return getAllFiles(files).map((file) => filesFromGit.indexOf(file) > -1);
} else {
   return getAllFiles(files);
}

(pseudocode to ilustrate it more or less)

@Djaler
Copy link
Contributor

Djaler commented Jul 2, 2019

What about merging the results?

@bartekleon
Copy link
Member Author

@Djaler merging the results?

@Djaler
Copy link
Contributor

Djaler commented Jul 2, 2019

I mean reports

@bartekleon
Copy link
Member Author

I don't quite understand @Djaler what you mean :/

@Djaler
Copy link
Contributor

Djaler commented Jul 2, 2019

I mean that by running stryker only for changed files you will get report only for this files. But we need to see the whole picture

@bartekleon
Copy link
Member Author

bartekleon commented Jul 2, 2019

@Djaler So we have two options...

  1. Combine reports - it will be super hard to achieve (at least thinking about it right now)
    combining reports would have to get some metadata of previous mutation (JSON or another format),
    then it should replace data of files that were changed, then it should again calculate mutation score for folders these files were in
  2. Do not combine reports - it has to be fast check, so let's check and give output only for files that were changed (then the user can have two stryker scripts 'full' and 'git optimized')

But you have a point

I think the second option is a better one since, for example, user can by hand using mutate to mutate files for example only from src/hello directory while skipping all other from src. So basically we do it for him :/ just removing all files that haven't been changed from report

@Djaler
Copy link
Contributor

Djaler commented Jul 2, 2019

I think this is related to #322

@bartekleon
Copy link
Member Author

It is slightly, yea. I think we should finish #1514 for now and maybe try this, at least using point '2' so we have an experimental version of this feature, and then we can try changing '2' into '1' :)

@nicojs
Copy link
Member

nicojs commented Jul 4, 2019

Great discussion here ❤️

The problem with only running Stryker on changed files can not always be trusted. Let's say you only change test files in a PR. There is no way to verify exactly which mutatations should be retested. So it would only works for changes made to source files to be mutated.

I think it's pretty easy to add a small part of code to your stryker.conf.js file to do this. Or use the stryker-diff-runner by @tverhoken that does exactly this.

Combine reports - it will be super hard to achieve (at least thinking about it right now)

I think this is the easy part. In fact, we've created the mutation-testing-report-schema with exactly this goal in mind. It's basically merging the files json object. This is an example:

https://github.com/stryker-mutator/mutation-testing-elements/blob/86bd8050f7e0bd9748b97237e26d596407331266/packages/mutation-testing-report-schema/testResources/strict-report.json#L7-L43

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature request New feature request
Projects
None yet
Development

No branches or pull requests

3 participants