--all appears to be broken in 5.1.0

[Lalem001]

In [email protected], I am getting the following error whenever --all is used:

user$nyc --all ava 'test/**/*.js'
fs.js:549
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
                 ^

Error: ENOENT: no such file or directory, open '/Users/user/project/.nyc_output/7910.json'
    at Error (native)
    at Object.fs.openSync (fs.js:549:18)
    at Object.fs.writeFileSync (fs.js:1156:15)
    at NYC.writeCoverageFile (/Users/user/project/node_modules/nyc/index.js:259:6)
    at NYC.addAllFiles (/Users/user/project/node_modules/nyc/index.js:171:8)
    at Object.<anonymous> (/Users/user/project/node_modules/nyc/bin/nyc.js:122:23)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)

Not sure where to start looking on this issue.

[bcoe]

@Lalem001 do things work on 5.0.1?

[Lalem001]

@bcoe yes.

I just experimented a bit and removing writeCoverageFile from addAllFiles stops the error, but I don't know yet if that changes the final output.

[bcoe]

CC: @jamestalmage any thoughts on this?

[Lalem001]

Final output is affected, of course, without writeCoverageFile. Will keep experimenting.

[jamestalmage]

Looks like it needs nyc._createDatastoreDirectories() here.

That used to create the cache and temp directories, but now it is back to just creating temp. So the name can probably be returned to createTempDirectory()

Needs further investigation, but I suspect #112 is the reason this wasn't caught by our tests.

[Lalem001]

From what I can tell the issue is caused by a missing .nyc_output directory.

If I disable NYC.cleanup, and manually create the directory, --all works fine.

Looks like @jamestalmage beat me to it.

[jamestalmage]

@Lalem001 - you are correct - see my comment above. You would not want to disable cleanup, but rather make sure the temp directory gets created.

[Lalem001]

@jamestalmage Disabling cleanup was just a step to figuring out the issue.

[bcoe]

@Lalem001, @jamestalmage cool -- this is why we release to a next branch first. Let's get a test around this behavior before we push 5.1.1 to next; perhaps we can just solve the cleanup ticket like @jamestalmage suggests.

[Lalem001]

_createDatastoreDirectories is called on new NYC.
nyc.cleanup is called afterwards by the bin.
To then call _createDatastoreDirectories again seems a bit redundant.

Perhaps we could add a cleanup option to NYC constructor and cleanup before _createDatastoreDirectories?

[jamestalmage]

Or, let's create a reset() function that is:

NYC.prototype.reset = function() {
  this.cleanup();
  this._createDatastoreDirectories();
}

Then, in the CLI, only use reset() instead of cleanup(). This prevents us from calling _createDatastoreDirectories() needlessly in every fork, which should give us a minor performance boost.

Feel free to come up with a better name than reset. 😄

[Lalem001]

@jamestalmage provision?

[jamestalmage]

provision?

I don't know. I think I might like reset better (though I'm still not thrilled with either).

Let's see if @novemberborn or @bcoe come up with anything.

[bcoe]

my vote would be for cleanup, let's get a pull for this in a branch that also resets state between tests -- @jamestalmage did you want to tackle this, or @Lalem001?

[jamestalmage]

So just add directory creation to the end end of the existing cleanup function?

[novemberborn]

So just add directory creation to the end end of the existing cleanup function?

Shouldn't cleanup, you know, clean up? Maybe the NYC constructor shouldn't create the directories. bin/nyc should, and anything else that calls NYC directly.

[jamestalmage]

Shouldn't cleanup, you know, clean up?

That was my thought with naming it reset() it seemed more appropriate (though definitely not the perfect name).

[jamestalmage]

See #121. I just named it reset. Happy to change it if someone has better ideas.