Add reset, clean up before every test

[jamestalmage]

This adds a reset method which deletes the temp directory then adds it back. createTempDirectory is no longer called for every forked process.

Instead of writing a test for #120, I just went ahead and fixed #112. I have verified it would have caught the error.

Fixes #120
Fixes #112

[novemberborn]

I wonder if the if (!process.env.NYC_CWD) clause in cleanup() is causing some confusion here. I think it ensures child processes don't perform any cleanup, but maybe that guard should be in this file and not in the implementation.

TBH I'm not entirely sure what the expected behavior is and why it impacts --all.

[jamestalmage]

The issue with all was that it was calling cleanup(), which deleted directories, but did not recreate them.

but maybe that guard should be in this file and not in the implementation.

I think I agree with you (I considered that as well). I just wanted to avoid an additional change creeping into this PR.

[novemberborn]

The issue with all was that it was calling cleanup(), which deleted directories, but did not recreate them.

Where did it call cleanup()? That's the part I'm not understanding, I suppose.

I just wanted to avoid an additional change creeping into this PR.

Sure.

[jamestalmage]

Where did it call cleanup()?

Right there in that diff. Now instead it calls reset, which deletes and recreates the directory.

The issue is that all immediately and synchronously writes a "preample" for every included file to disk, before continuing with the test, so the folder needs to exist, even though we haven't yet forked a process.

[novemberborn]

Ah! OK, resetting before running anything makes sense.

[bcoe]

@jamestalmage how do you feel about moving the cache folder into .nyc_output? I think it would be nice if we only created one top-level folder with nyc, rather than a top-level folder and a folder in node_modules -- I probably should have brought this up in the earlier code review :)

[jamestalmage]

Separate folders makes it easier to clean between tests (we don't want to clean the cache each time).

Also, AVA has already followed suit and caches Babel transforms in node_modules/.cache/ava.

If anything, I would like to move .nyc_output into node_modules/.temp/nyc or similar. Then people would not need to modify their .gitignore for nyc.

[bcoe]

@jamestalmage this seems like a reasonable argument, it's good that AVA and nyc are standardizing on an approach.

Let's hold off on moving .nyc_output at this point, I'd like to get a stable 5.1.1 out the door and tested for a bit, before we make many more changes.

[bcoe]

I'm happy with this branch, once you and @novemberborn both sign off.

[novemberborn]

I don't quite understand why --all is broken and how this fixes it, but no objections otherwise.

[bcoe]

@Lalem001, can you confirm that this branch solves your --all problem?

[Lalem001]

The issue is resolved. 👍

[bcoe]

[bcoe]

@Lalem001 [email protected] is now out with caching, and fixes for --all.