istanbuljs / test-exclude Goto Github PK

Consider upgrading glob to v9 which has significant performance improvements —
https://github.com/isaacs/node-glob/blob/main/changelog.md

Or better use something like fast-glob which is built for speed.

Currently, calculating test-exclude on large projects can take several seconds.

See description here: bcoe/c8#183

Capital vs lowercase driver letters can throw this check off as it is currently written.

CC: @bcoe

Minimatch is flagged as HIGH vulnerability in PRISMA. Fix version found in 3.0.5.

PRISMA issue link: isaacs/minimatch@a8763f4

Can you please check and let me know if more info required?

It's surprising behavior that when you set a custom exclude rule you almost always need to add node_modules. We should instead using a negated glob if folks want to instrument node_modules.

See the discussion with @sindresorhus in istanbuljs/nyc#348

This is the culprit behind: istanbuljs/nyc#1381

I'll open a PR about it soon

Hello,
text-exclude currently requires Glob 7, which includes Inflight, which has a security issue: CWE-772. Inflight will not be fixed and the developer encourages everyone to upgrade to Glob 9 or 10, which don't have the dependency. Currently, overriding Glob to use 10 causes test-exclude to throw an error when used in Jest.

TypeError: The "original" argument must be of type function. Received an instance of Object

      at Object.<anonymous> (node_modules/test-exclude/index.js:5:14)
      at Object.<anonymous> (node_modules/babel-plugin-istanbul/lib/index.js:18:43)

This would cause tests in Jest to break if developers try to mitigate the security bulletin.
Using Glob 9 currently works, but then other packages break, so it would be great if test-exclude could use the latest.

in the 'index.js' file
in the function shouldInstrument

if (
      this.extension &&
      !this.extension.some(ext => filename.endsWith(ext))
  ) {
      return false;
  }

sometimes filename is undefined then report error

Can it be changed to !this.extension.some(ext => filename && filename.endsWith(ext))

This case is from istanbuljs/nyc#554. Reopened here as suggested by @bcoe. Anything mentioned there applies. All files listings and version info located at Gist. More specifics follows:

Problem: directory junctions on windows systems do not excluded from paths correctly.

Description & Case:

First, a structure example. Here, dir is simple directory, while junc is junction to dir (made with mklink /j junc dir)

`-- expr
  +-- dir
  | `-- foo.js
  +-- junc >> dir
  +-- main.js
  `-- package.json

main.js is simple test:

const assert = require('assert')

const exclude = require('test-exclude')

const junctionOnly = exclude({
  exclude: ['dir/']
})

assert.strictEqual(false, junctionOnly.shouldInstrument('dir/foo.js')) // GOOD
assert.strictEqual(true, junctionOnly.shouldInstrument('junc/foo.js')) // GOOD

const plainDirOnly = exclude({
  exclude: ['foo/']
})

assert.strictEqual(true, plainDirOnly.shouldInstrument('dir/foo.js')) // GOOD
assert.strictEqual(false, plainDirOnly.shouldInstrument('junc/foo.js')) // BAD

You can see that assert fails at excluding junctioned path.

I've seen a couple nyc bugs where configuration: {include: ['./lib/**']} does not work, it has to be changed to {include: ['lib/**']}. These two include arrays should produce identical results.

TextExclude.shouldInstrument(filename) returns false when options.relativePath is true and filename is not inside options.cwd.

However:

• when options.cwd is a symlink and is passed with its symlink unresolved
• but filename resolves the symlink (through fs.realpath() for example)
• then shouldInstrument() will return false even though it should return true.

This is likely to happen since:

• the filename passed by nyc always has symlinks resolved because that's how module._compile() works
• however nyc user might pass a options.cwd that has an unresolved symlink.

Real-life example: running the following on Travis on Mac OS:

const os = require('os');
const execa = require('execa');

const cwd = os.tmpdir();
await execa.shell(`nyc --cwd ${cwd}`)

Travis Mac OS environment:

• returns /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T with os.tmpdir()
• but this directory is a symlink for /private/var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T
• as a consequence all filename passed to testExclude.shouldInstrument() will start with /private/var/..., while options.cwd() will start with /var/..., even though it's the same folder.

One possible solution would be to run fs.realpath() on both filename and options.cwd() before comparing them.

Using node:latest and "nyc": "^15.1.0" bumps into the following error due to your library:

./node_modules/.bin/nyc --reporter=lcov --reporter=text ./node_modules/.bin/mocha tests --timeout 10000 --recursive --check-leaks --exit  --reporter mocha-junit-reporter --reporter-options mochaFile=test_reports/mocha/test-results.xml
node:internal/modules/cjs/loader:927
  throw err;
  ^

Error: Cannot find module 'read-pkg-up'
Require stack:
- /root/workspace/node_modules/test-exclude/index.js
- /root/workspace/node_modules/nyc/lib/config-util.js
- /root/workspace/node_modules/nyc/bin/nyc.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:924:15)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/root/workspace/node_modules/test-exclude/index.js:4:19)
    at Module._compile (node:internal/modules/cjs/loader:1092:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Module.require (node:internal/modules/cjs/loader:996:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/root/workspace/node_modules/test-exclude/index.js',
    '/root/workspace/node_modules/nyc/lib/config-util.js',
    '/root/workspace/node_modules/nyc/bin/nyc.js'
  ]
}

Exited with code exit status 1

./node_modules/.bin/nyc --reporter=lcov --reporter=text ./node_modules/.bin/mocha tests --timeout 10000 --recursive --check-leaks --exit  --reporter mocha-junit-reporter --reporter-options mochaFile=test_reports/mocha/test-results.xml
node:internal/modules/cjs/loader:1109
      throw new ERR_REQUIRE_ESM(filename, parentPath, packageJsonPath);
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /root/workspace/node_modules/read-pkg-up/index.js
require() of ES modules is not supported.
require() of /root/workspace/node_modules/read-pkg-up/index.js from /root/workspace/node_modules/test-exclude/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /root/workspace/node_modules/read-pkg-up/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /root/workspace/node_modules/read-pkg-up/package.json.

    at new NodeError (node:internal/errors:329:5)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1109:13)
    at Module.load (node:internal/modules/cjs/loader:972:32)
    at Function.Module._load (node:internal/modules/cjs/loader:813:14)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/root/workspace/node_modules/test-exclude/index.js:4:19)
    at Module._compile (node:internal/modules/cjs/loader:1092:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1121:10)
    at Module.load (node:internal/modules/cjs/loader:972:32) {
  code: 'ERR_REQUIRE_ESM'
}

Exited with code exit status 1

Any advice and insight is appreciated. sindresorhus/read-package-up#18

• The first argument of shouldInstrument needs to be an absolute filename.
• Need to document excludeNodeModules: false as the preferred method of not excluding node_modules.
• Overall need more complete API documentation. Can we use JS docs or something to generate documentation?

i wonder why there has to be a check for a not-empty array over here
https://github.com/istanbuljs/test-exclude/blob/master/index.js#L21

  if (!this.exclude || (Array.isArray(this.exclude) && !this.exclude.length)) {
    this.exclude = [
      'test/**',
      'test{,-*}.js',
      '**/*.test.js',
      '**/__tests__/**'
    ]
  }

i'm trying to test the instrumentation code and obviously it gets excluded :)
All i'm trying to do is to configure the babel-plugin-istanbul to not exclude anything

babel.transform(content, {
  filename,
  plugins: [[babelPluginIstanbul, {exclude: []}]],
}).code;

but because of this check, the excludes get their default value.

can it be just

if (!Array.isArray(this.exclude)) {
  //  this.exclude = ...
}

It seems this project will not allow any node_modules, not even ones local to a project. Given how node_modules works, why disallow any coverage of code there?

I can help. Please let me know if there is any positive interest in this.