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

feat(reload test environment): implement test environment reload #3369

Merged
merged 4 commits into from Jan 28, 2022

Conversation

nicojs
Copy link
Member

@nicojs nicojs commented Jan 20, 2022

Add the ability for Stryker to restart a test runner in order to reload its test environment. This is used to test static mutants where the test runner is incapable of reloading the test environment by itself. This is a requirement to support es modules (ESM) since it is impossible to reload an ESM using something like delete require.cache['...']. Instead, Stryker will restart the entire test runner process for those mutants.

The way for the test runner to communicate to Stryker whether it is capable of reloading the environment is by implementing the new capabilities() method.

capabilities() {
  return { reloadEnvironment: true }
}

If you do this, you can expect the new reloadEnvironment boolean in your MutantRunOptions to be true when you need to reload the test environment after activating the mutant.

See #2413 and #2922

I also moved some code around (from the MutationTestProcess to a new class MutantTestPlanner).

BREAKING CHANGE: Test runner plugins must provide TestRunnerCapabilities by implementing the capabilities method.

Add `capabilities()` to test runner api as a required method

BREAKING CHANGE: Test runner plugins are now required to provide `TestRunnerCapabilities`.
@nicojs
Copy link
Member Author

nicojs commented Jan 21, 2022

@simondel do you think it is OK to make capabilities() required?

@nicojs
Copy link
Member Author

nicojs commented Jan 27, 2022

This change also contains a breaking change for the reporter API:

- export type MutantTestCoverage = Mutant &
-  Pick<schema.MutantResult, 'coveredBy' | 'static'> & {
-    estimatedNetTime: number;
-    hitCount?: number;
-    testFilter: string[] | undefined;
-  };
+ export type MutantTestCoverage = Mutant & Pick<schema.MutantResult, 'coveredBy' | 'static'>;

I've removed estimatedNetTime, hitCount, and testFilter. They weren't used by our reporters and they were only there to pass along state internally in core, between the find-mutant-test-coverage and mutation-test-executor. I found this very obfuscated and it frustrated me.

That's why I now choose to do all the work in the new MutantTestPlanner class. It is responsible for planning which mutant should be run with which options and which should be reported as an early result.

We can in a later PR replace the onAllMutantsMatchedWithTests with a new onMutantPlanDetermined, which can include the run options per test. That way we can improve the progess reporter. Instead of counting the same progress per mutant, it can count mutants that require a test env reload 3 times for example.

I want to document the breaking in change in that PR, so we don't get a big mess in our changelog.


public makePlan(mutants: readonly Mutant[]): readonly MutantTestPlan[] {
const mutantTestPlans = mutants.map((mutant) => this.planMutant(mutant));
this.reporter.onAllMutantsMatchedWithTests(mutantTestPlans.map(({ mutant }) => mutant));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking, but I like mutantTestPlans.map(testPlan => testPlan.mutant) more

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, a nitpick 😉

// If not static, or it was "hybrid" (both static and perTest coverage) and ignoreStatic is on.
// Only run covered tests
const estimatedNetTime = calculateTotalTime(tests);
return {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to have a factory/create method for these MutantTestPlan objects because current function is quite long

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created factory methods for early result as well as for mutant plan

return testsByMutantId;
}

function calculateTotalTime(testResults: Iterable<TestResult>): number {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just a reduce function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it is called with both a Set as an Array. I didn't want to create a bogus array just to be able to call reduce. This is better.

return total;
}

function toTestIds(testResults: Iterable<TestResult>): string[] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just a map function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. Note that reduce and map might be coming to Iterables in an ES specification near you: https://github.com/tc39/proposal-collection-methods#proposal

@nicojs nicojs merged commit b95b907 into epic/esm Jan 28, 2022
@nicojs nicojs deleted the feat/test-capabilities branch January 28, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants