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

fix(StrykerTempFolder): Use local tmp folder #121

Merged
merged 4 commits into from
Jul 19, 2016
Merged

Conversation

nicojs
Copy link
Member

@nicojs nicojs commented Jul 12, 2016

When running mocha tests, we need to be able to resolve the local node_modules. The way this works in npm is that it searches for a node_modules in de current working directory. If not found, check the dir above. Etc. By creating the tmp-folder's locally, node_,modules will be resolved as expected.

When running mocha tests, we need to be able to resolve the local node_modules. The way this works in npm is that it searches for a node_modules in de current working directory. If not found, check the dir above. Etc. By creating the tmp-folder's locally, node_,modules will be resolved as expected.
@simondel
Copy link
Member

When will this folder be removed form the file system? We can assume that a temp folder will (eventually) be cleared, but this is not the case for other locations such as your project folder.

@nicojs
Copy link
Member Author

nicojs commented Jul 12, 2016

Hmm that's true. Haven't thought about the cleaning ♻️

What's your suggestion? I think we can just clear the entire StrykerTempFolder after stryker is done. But we could also make each individual part responsible for cleaning its own stuff.

@simondel
Copy link
Member

I would use a 'fire and forget' method for saving files. I don't believe that classes such as the TestRunnerOrchestrator should care if they make a mess in the StrykerTempFolder. I would let Stryker remove the temp folder before it exits.

One disadvantage of the StrykerTempFolder is that it will exist if users kill Stryker, but by making Stryker just delete the entire folder after a run, a cleanup will still happen.

@nicojs
Copy link
Member Author

nicojs commented Jul 12, 2016

Agreed. I'll implement that in this branch.

You always run the risk of having temp-files remain if you kill the process, that's totally acceptable.

One additional disadvantage is that you cannot run multiple instances of Stryker at one time anymore, because the one that is the first to close will clean all files. But i don't think its a big issue.

@nicojs
Copy link
Member Author

nicojs commented Jul 12, 2016

@simondel we should first merge pull request 116 into master, that one contains fileUtils to easily clean a folder recursively.

@nicojs
Copy link
Member Author

nicojs commented Jul 19, 2016

I just added the cleaning of the folder ♻️ !

@simondel simondel merged commit 84790f2 into master Jul 19, 2016
@simondel simondel deleted the local-tmp-folder branch July 19, 2016 05:37
nicojs added a commit that referenced this pull request Jul 30, 2016
* fix(StrykerTempFolder): Use local tmp folder

When running mocha tests, we need to be able to resolve the local node_modules. The way this works in npm is that it searches for a node_modules in de current working directory. If not found, check the dir above. Etc. By creating the tmp-folder's locally, node_,modules will be resolved as expected.

* fix(stryker-temp): Clean temp folder

* fix(delete-dir): Fix for deleting a dir recrusive
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.

2 participants