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

test-exclude doesn't support absolute paths #34

Open
stevenvachon opened this issue Mar 6, 2019 · 3 comments
Open

test-exclude doesn't support absolute paths #34

stevenvachon opened this issue Mar 6, 2019 · 3 comments

Comments

@stevenvachon
Copy link

const testExclude = require('test-exclude');
const exclude = testExclude({
  exclude: ['/project/file']
});

exclude.shouldInstrument('./file')); //-> false
@coreyfarrell
Copy link
Member

Just to make sure I understand what you are saying, can I assume that your above test assumes that process.cwd() === '/project'?

const assert = require('assert');
const path = require('path');
const testExclude = require('test-exclude');

function failingTest() {
  const exclude = testExclude({
    exclude: [path.resolve('file')]
  });
  assert.equal(exclude.shouldInstrument('./file'), false);
}

function passingTest() {
  const exclude = testExclude({
    exclude: [path.resolve('file')],
    relativePath: false
  });
  assert.equal(exclude.shouldInstrument(path.resolve('./file')), false);
}

// a call to `failingTest()` will assert, a call to `passingTest()` will not.

There are issues mixing of relative and absolute paths, I'm not aware of anything that uses relativePath: false outside test-exclude's own tests.

@stevenvachon
Copy link
Author

stevenvachon commented Mar 8, 2019

Correct, cwd is /project.

relativePath: false is required to make it work, or will that make relative paths fail ("mixing") ?

@coreyfarrell
Copy link
Member

Mixing does not work as of now. The only correct way (as of now) to exclude ./file is exclude: ["file"].

exclude.shouldInstrument() actually expects an absolute path to be provided for the first argument. If relativePath: true it converts to a relative path for comparison with the include/exclude options.

It's a tough situation, we have to be really careful with logic changes to test-exclude (4.8 million downloads last week, lots of users dependent on the current logic). By far the nominal use is relativePath: true.

Side note: it appears that the README for test-exclude is incorrect as the examples show passing relative filenames to shouldInstrument. I've opened #33 regarding the need to correct/improve the documentation.

@coreyfarrell coreyfarrell transferred this issue from istanbuljs/istanbuljs Sep 30, 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

No branches or pull requests

2 participants