Feature: use git to identify files (instead of the files array)

[nicojs]

Current problem

Right now, a lot of reported issues come from people misunderstanding what the files array does in your stryker.conf.js files. And who can blame them. It's kind of a mess.

Here is a list of the most common issues:

1. "Why do I need to specify files?"
2. "Why do I need to put !**.* as the first argument?" (psst, both stryker-typescript and stryker-karma-runner expand the files array even before your config is loaded, resulting in a mess. I know, complicated indeed!)
3. "Why do i need to add insert-file-name-here to my files?"
4. "What is the difference between mutate and files?"

We can do better than this.

Feature request: use git

Let's replace the current functionality with using the project's .gitignore file by default. This is much more reliable and user-friendly than any custom solution we can come up with. ~For users that still need to specify the files, we can use a .strykerignore file construction. It will work the same way as .gitignore works, so no need to learn something new.~ EDIT: We won't use a .strykerignore file, instead we still support a files array with strings for which files to include.

Way of working

The new way of working will be:

1. Use files array if present.
2. If not: use .gitignore file. Use the output of the git ls-files --others --exclude-standard --cached command to get the list without having to parse the .gitignore files ourselves.
3. ~If not in a git repo (or git isn't installed), use a sane default (ignore only node_modules or something).~ EDIT: We shouldn't use a default. If we did that, the same project on different machines would result in different files being used and different overall result. Instead, we should throw an error which clearly specifies that the input files could not be specified

Advantages:

This way of working has a lot of advantages

1. It will be zero config for most users. Users don't even have to know about sandboxes or anything.
2. Marking files to be mutated will be easier, as you can only do that by specifying them in the mutate array. No more mix and matching by combining the files and mutate array.

Downsides

We have some downsides to this new approach.

1. It is a major (breaking) change
2. Sandboxes will be bigger than they would need to be. This will not be a big problem I think.
3. We will lose support for additional file properties you can now specify with your files: transpiled, included and mutated. We can mitigate this:
   • mutated: This one is simple: use the mutate property instead.
   • included: This should move to the test runner configuration (with sane defaults). This might give some problems with both the mocha and karma test runners.
   • transpiled: This should be moved to the transpiler configuration. Not a big deal, as all transpilers have their own config anyway (webpack.config.js, tsconfig, .babelrc, etc)

Todo list:

Work on this feature is done in the feat/identify-files branch

• [x] stryker-api: Remove the FileDescriptor, TextFile, WebFile and BinaryFile interfaces. Add the File class which represents a binary or text file.
• [x] stryker: add "downgrade" from old InputFileDescriptor objects to strings with a deprecation message.
• [x] stryker: Don't input file resolver: don't read deleted files
• [x] stryker: Support coverage analysis of typescript projects
• [x] stryker: Update the InputFileResolver class to now use the new way of identifying files
• [x] stryker: Update the integration with Transpiler and TestRunner.
• [x] stryker: Serialize/deserialize the new File class instance using surrial
• [x] stryker-typescript: Remove the feature of magically adding files from the TypescriptConfigEditor
• [x] stryker-typescript: Replace the feature of using the transpiled property, instead use the files defined in the tsconfig.json file directly
• [x] stryker-karma-runner: Remove the feature of magically adding files from the StrykerKarmaEditor
• [x] stryker-karma-runner: Add functionality of using the files array from the karma.conf.js file.
• [x] stryker-karma-runner: Make sure the testHooks is always added (first file).
• [x] stryker-babel-transpiler: Don't use the transpiled property anymore
• [x] stryker-mocha-runner: Don't use the included property anymore: instead use the mocha opts (or default to test folder as is the default for mocha)
• [x] stryker-mocha-runner: Make sure the testHooks is always evaluated.
• [x] stryker-mocha-runner: Fix this issue:

  [2018-03-23 10:00:58.113] [TRACE] TestRunnerChildProcessAdapter - [2018-03-23 10:00:58.114] [DEBUG] IsolatedTestRunnerAdapterWorker - UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: No files discovered (tried pattern(s) [
    "test/*.js"
  ]). Please specify the files (glob patterns) containing your tests in mochaOptions.files in your stryker.conf.js file.

• [x] stryker-javascript-mutator: Implement new api.
• [x] stryker-webpack-transpiler: Implement new api.
• [x] documentation: Document the new way of working.
• [x] update readme's
• [x] update initStrykerConfig keys in package.json's

[nicojs]

@simondel @Archcry let's take some time to think about this proposal before we start to implement it. I don't want to rush it, because it will take a chunk of time to implement and is breaking for end users.

[sanderkoenders]

Wouldn't it be possible to still support the files array and use the proposed method when the files array is not specified (undefined)? That way we stay backwards compatible (no breaking change).

[nicojs]

Hmm interesting. In that case I don't want to support a .strykerignore file as well, as both would serve the same purpose. I would also still want to get rid of the additional file attributes (transpiled, included, mutated) as those add to the confusion.

An additional benefit is that keeping the files array is simpler to implement, as we keep the current functionality. I was looking for a module that delivers the .blaignore file functionality, but it seems it does not exist, so keeping the files array makes even more sense to me.

However, we should remove the "magic" adding of files from the ConfigEditor plugins.

@Archcry do you agree with all this?

[sanderkoenders]

Ye I agree, magic causes side effects.

[nicojs]

I've updated the original issue, is this what we want @Archcry @simondel ?

[nicojs]

@simondel the more I think about it, the happier I get. When we are doing implementing this feature we can also change the strange WebFile interface we now have. It is basically only needed to support karma's url-files. We should be able to simply remove it.

We might also be able to remove the distinction between binary and text files. We can simply read any file as buffer. If we need the text representation, we can use buffer.toString('utf8') (as a cached property). This would simplify a lot of code.

[sanderkoenders]

But how will we know what files to mutate in the new situation?

[nicojs]

We will still support the mutate property

[sanderkoenders]

I understand, is it also possible to ignore certain files with this property? Because when you have two files (file.js & fileSpec.js) you can't simply say **/*.js.

[nicojs]

Yes you can exclude with a ! just like you can with files.

[nicojs]

I started the work on the https://github.com/stryker-mutator/stryker/tree/feat/identify-files branch. In order to keep a green build during the breaking changes, we will scope the build on only the packages that are updated (as of now, only the stryker-api package is updated)

[simondel]

We may be able to rewrite the TypescriptConfigEditor to read the ts files for us that have to be mutated.

[nicojs]

@simondel

We may be able to rewrite the TypescriptConfigEditor to read the ts files for us that have to be mutated.

That's not what we want I think. Let's focus on removing the 'hocus pocus' configuration. It will only over complicate things.