stryker-mutator / stryker-api Goto Github PK

i have karma.conf.js with:
preprocessors: {
'**/*.html': ['ng-html2js']
},

https://github.com/karma-runner/karma-ng-html2js-preprocessor

it used to combine angular1 html templates into one js file and put into angular template cache
we activly using this preprocessor for our unit tests (also we have another 1 our custom preprocessor)

Seems stryker skipped karma preprocessors, and i create small workaround to create separate file with angular templates before runnig stryker

It will be much simplier to support karma preprocessors from karma.conf.js or from overrided config in stryker.conf.js

I want to discus a better test runner API. Right now it looks like this (removed jsdoc for clarity):

 // Omitted some details for clarity

namespace TestRunnerFactory {
  class TestRunnerFactory extends Factory<RunnerOptions, TestRunner> {
  }
  let testRunnerFactoryInstance = new TestRunnerFactory();
  export function instance() {
    return <Factory<RunnerOptions, TestRunner>>testRunnerFactoryInstance;
  }
}

interface TestRunner {
  run(options: RunOptions): Promise<RunResult>;
}

interface RunResult {
  result: TestResult;
  testNames?: string[];
  succeeded?: number;
  failed?: number;
  timeSpent?: number;
  coverage?: CoverageCollection;
  errorMessages?: string[];
}

enum TestResult {
  Complete,
  Error,
  Timeout
}

I have a few problems with this.

1. We cannot see which tests failed and which tests passed.

When we have failed tests, we cannot see which of them failed and which of them passed. We also cannot see which error message belonged to which test. We could solve this by creating a new entity to hold the result of one single test. Something like this:

interface RunResult {
  testResults: TestResult[];
  timeSpent: number;
  coverage?: CoverageCollection;
}

interface TestResult{
  name: string;
  state: TestState;
  errorMessages?: string[];
}

enum TestState { Complete, Error, Timeout }

This can work for karma and mocha, which both know which tests are being ran. Which brings me to my next point.

2. Implementing a test runner is kind of difficult.

When implementing a test runner you have to provide a lot of information. You need to report code coverage (it will work without it, but there is know way to communicate this to Stryker in advance), you need to report on the tests being ran and the state of each test with possible error. This is too bad, if you for example don't use an out-of-the-box test runner at all, but your custom testing logic.

What would the best course of action be here? We could change the interface of the TestRunnerFactory , in that it requires a static method capabilities(): TestCapabilities on each test runner. The test capabilities could look like this:

interface TestCapabilities {
  codeCoverage?: boolean;
  identitfyTests?: boolean;
}

Stryker could then use this interface to determine the best strategy to run mutations.

This brings me to my third point:

3. Implementing code coverage reporting is a lot of work.

Implementing code coverage reporting is a lot of work. Just think about it. A test runner needs to know which code it must instrument for code coverage (the once with mutated: true), then it needs to instrument the code for code coverage and it has to do that with istanbul (or transform the code coverage report of the coverage tool in question). Finally, it needs to report the code coverage object.

Could we find a better way to implement code coverage? Maybe let Stryker instrument the code and then leave it up to the test runner to report the code coverage report (and reset it for next run). Maybe not ideal, as the code coverage responsibility gets filled by a tandem of Stryker and the test runner.

Can anyone think of a solution?

We should be able to merge the Config class and StrykerOptions classes.

They are both quite similar. StrykerOptions has basically the same interface as Config, but optional. We can use awesome ts2.1 mapped type Partial<Config> for this:

https://github.com/Microsoft/TypeScript/blob/0a535f0bf7193741e6b4acf5b7dfea88e2d4beca/lib/lib.d.ts#L1379-L1384

One problem is the set function. I actually don't like it. Could we get rid of this? Why do we need it? We could make our config simpler:

module.exports = {
    // Stryker config
}

... so no function(config){ config.set({ ... })} anymore.

I propose to do this in 2 phases.

1. Make the set method deprecated. Support both versions of config.
2. Remove it in a future version.

@simondel what do you think?