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

Custom cache directory should be resolved to an absolute path #765

Closed
mojavelinux opened this issue Jan 22, 2018 · 6 comments
Closed

Custom cache directory should be resolved to an absolute path #765

mojavelinux opened this issue Jan 22, 2018 · 6 comments

Comments

@mojavelinux
Copy link
Contributor

Expected Behavior

I expect nyc to consistently use the same cache directory throughout the process. When a custom cache directory is specified as a relative path, and the working directory of the process changes, this assumption is violated.

The custom cache path needs to be resolved to an absolute path (using path.resolve()) before it's used.

Observed Behavior

If a custom cache directory is specified as a relative path (such as in package.json), and the working directory of the process is changed by one of the tests, then the cache directory will be recreated in a new location for that portion of the test suite. This puts cache files in an unexpected location and defeats the purpose of having a cache.

If the cache directory is left unspecified, then nyc correctly resolves the value to an absolute path. It's the custom path that isn't being resolved.

See: https://github.com/istanbuljs/nyc/blob/master/index.js#L52

This line needs to be:

this.cacheDirectory = path.resolve(config.cacheDir) || findCacheDir({name: 'nyc', cwd: this.cwd})

Forensic Information

Operating System: Fedora 26 (Linux)
Environment Information:

Node: v8.9.3
npm: 5.5.1
yarn: 1.3.2
nyc: 11.4.1

@mojavelinux
Copy link
Contributor Author

mojavelinux commented Jan 22, 2018

This is easily reproducible by simply changing the working directory of the process in a test:

describe('cache directory problem', () => {
  it('should put .nyc_cache in different folder', () => {
    process.chdir('another_folder')
    // require some code that nyc is going to cache
  })
})

Naturally, this only happens if the cache operation is triggered after the working directory change...so we're talking code which is loaded dynamically.

@bcoe
Copy link
Member

bcoe commented Jan 22, 2018

@mojavelinux agreed, this sounds like a bug; any interest in submitting a failing test?

@mojavelinux
Copy link
Contributor Author

Already did. Check the PR. #766.

@mojavelinux
Copy link
Contributor Author

As a bonus, I added a test for both cases.

@mojavelinux
Copy link
Contributor Author

I discovered this bug when my test suite started throwing errors trying to write the nyc cache files. Since the cache folder was moving, it was getting caught up in the clean up script that runs in the beforeEach of my suite. When nyc tried to write to the cache folder, it was gone.

@coreyfarrell
Copy link
Member

Thanks for the fix, looks like this released in nyc@11.5.0. If I'm mistaken please let me know and I can reopen this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants