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

feature(stryker): #336 Configurable tmp folder location #1644

Merged
merged 15 commits into from Sep 5, 2019

Conversation

brunoqueiros
Copy link
Contributor

Ability to specify a different temporary folder instead of .stryker-tmp. Solution based on #475

stryker —tempDir=.another-path-tmp

@nicojs
Copy link
Member

nicojs commented Jul 17, 2019

I've made some changes. 2 things mainly:

  • Changed provideValue to provideClass. With typed-inject's provideValue('temporaryDirectory', TemporaryDirectory) you specify to provide the class itself for the 'temporaryDirectory' token. Using provideClass will instruct typed-inject to instantiate and cash the temp dir.
  • Removed the instance() method, since typed-inject will now make sure it is a singleton.

We could still improve on the dependency injection logic. Especially the passing around of the temp dir variable across multiple constructors. But it is already a big improvement.

I didn't update the unit tests or anything, you want to take that? 😅

@brunoqueiros brunoqueiros marked this pull request as ready for review July 19, 2019 09:11
@brunoqueiros
Copy link
Contributor Author

@nicojs what do you think? ☝️

@@ -256,10 +256,11 @@ describe(Stryker.name, () => {
expect(mutationTestExecutorMock.run).calledWith(mutants);
});

it('should clean the stryker temp folder', async () => {
// TODO: how to test it
xit('should clean the stryker temp folder', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed anymore. typed-inject is now responsible for the disposing of it.

@nicojs
Copy link
Member

nicojs commented Sep 4, 2019

@brunoqueiros I've made some changes. Namely:

  • Merge master back into this branch
  • Document the tempDirName option in readme and StrykerOptions / Config
  • Make unit tests pure unit tests instead of testing integration. I.e. use sinon.createStubInstance(TemporaryDirectory) instead of testInjector.injectClass(TemporaryDirectory) wen not unit testing TemporaryDirectory itself

I will merge it if the build is green. 💚

Sorry it took so long. I was on vacation and didn't get around to looking at this until now. 😥

@nicojs nicojs merged commit 301f288 into stryker-mutator:master Sep 5, 2019
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