glob-base icon indicating copy to clipboard operation
glob-base copied to clipboard

Consider updating is-glob, glob-parent to 3.x series

Open pravi opened this issue 9 years ago • 17 comments

When running tests with latest versions of glob-parent and is-glob,

git diff
diff --git a/package.json b/package.json
index 8cb2dbf..fe4d7cc 100644
--- a/package.json
+++ b/package.json
@@ -29,8 +29,8 @@
     "test": "mocha"
   },
   "dependencies": {
-    "glob-parent": "^2.0.0",
-    "is-glob": "^2.0.0"
+    "glob-parent": "^3.0.0",
+    "is-glob": "^3.0.0"
   },
   "devDependencies": {
     "mocha": "*",

5/10 tests fail, see error below. In debian, we like to keep the latest versions of any library and we already have glob-parent and is-glob at version 3.0.

$ npm test

> [email protected] test /home/pravi/forge/debian/git/pkg-javascript/node-glob-base
> mocha



  glob-base:
    1) typical globs:
    ✓ file extensions:
    2) negation pattern:
    ✓ extglobs:
    3) braces: no base:
    4) braces in filename:
    5) braces in dirname:
    ✓ regex character classes:
    ✓ qmarks:
    ✓ non-glob pattern:


  5 passing (46ms)
  5 failing

  1) glob-base: typical globs::

      AssertionError: expected { base: './{a/b/{c,', glob: 'foo.js}/e.f.g}', isGlob: true } to equal { base: '.', glob: '{a/b/{c,/foo.js}/e.f.g}', isGlob: true } (at base, A has './{a/b/{c,' and B has '.')
      + expected - actual

       {
      -  "base": "./{a/b/{c,"
      -  "glob": "foo.js}/e.f.g}"
      +  "base": "."
      +  "glob": "{a/b/{c,/foo.js}/e.f.g}"
         "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:38:50)

  2) glob-base: negation pattern::

      AssertionError: expected { base: 'a/b/c', glob: '!foo', isGlob: false } to equal { base: 'a/b/c', glob: '!foo', isGlob: true } (at isGlob, A has false and B has true)
      + expected - actual

       {
         "base": "a/b/c"
         "glob": "!foo"
      -  "isGlob": false
      +  "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:76:35)

  3) glob-base: braces: no base::

      AssertionError: expected { base: '/a/b/{c,', glob: 'foo.js}/e.f.g/', isGlob: true } to equal { base: '/a/b', glob: '{c,/foo.js}/e.f.g/', isGlob: true } (at base, A has '/a/b/{c,' and B has '/a/b')
      + expected - actual

       {
      -  "base": "/a/b/{c,"
      -  "glob": "foo.js}/e.f.g/"
      +  "base": "/a/b"
      +  "glob": "{c,/foo.js}/e.f.g/"
         "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:87:48)

  4) glob-base: braces in filename::

      AssertionError: expected { base: 'a/b/.{c,', glob: '.gitignore}', isGlob: true } to equal { base: 'a/b', glob: '.{c,/.gitignore}', isGlob: true } (at base, A has 'a/b/.{c,' and B has 'a/b')
      + expected - actual

       {
      -  "base": "a/b/.{c,"
      -  "glob": ".gitignore}"
      +  "base": "a/b"
      +  "glob": ".{c,/.gitignore}"
         "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:97:45)

  5) glob-base: braces in dirname::

      AssertionError: expected { base: 'a/b/{c,.', glob: 'd}/e/f.g', isGlob: true } to equal { base: 'a/b', glob: '{c,./d}/e/f.g', isGlob: true } (at base, A has 'a/b/{c,.' and B has 'a/b')
      + expected - actual

       {
      -  "base": "a/b/{c,."
      -  "glob": "d}/e/f.g"
      +  "base": "a/b"
      +  "glob": "{c,./d}/e/f.g"
         "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:105:42)



npm ERR! Test failed.  See above for more details.
npm WARN This failure might be due to the use of legacy binary "node"
npm WARN For further explanations, please read
/usr/share/doc/nodejs/README.Debian

npm ERR! not ok code 0

pravi avatar Oct 28 '16 12:10 pravi

ok, I'll make this change next as soon as I get a chance. A pr would also be great if someone beats me to it. thanks

jonschlinkert avatar Oct 28 '16 12:10 jonschlinkert

I'll try to make a pr.

pravi avatar Oct 28 '16 13:10 pravi

The failures have to do with the same circumstances discussed in https://github.com/es128/glob-parent/issues/10 and https://github.com/es128/glob-parent/pull/11.

For the record, if https://github.com/es128/glob-parent/pull/11 were merged and published, the tests here pass.

es128 avatar Oct 28 '16 14:10 es128

I initially thought only tests need fixing, but I'll wait for a fix in glob-parent.

pravi avatar Oct 28 '16 14:10 pravi

@es128 after applying es128/glob-parent#11, I test still fails,

 $ npm test
> [email protected] test /home/pravi/forge/debian/git/pkg-javascript/node-glob-base
> mocha



  glob-base:
    ✓ typical globs:
    ✓ file extensions:
    1) negation pattern:
    ✓ extglobs:
    ✓ braces: no base:
    ✓ braces in filename:
    ✓ braces in dirname:
    ✓ regex character classes:
    ✓ qmarks:
    ✓ non-glob pattern:


  9 passing (49ms)
  1 failing

  1) glob-base: negation pattern::

      AssertionError: expected { base: 'a/b/c', glob: '!foo', isGlob: false } to equal { base: 'a/b/c', glob: '!foo', isGlob: true } (at isGlob, A has false and B has true)
      + expected - actual

       {
         "base": "a/b/c"
         "glob": "!foo"
      -  "isGlob": false
      +  "isGlob": true
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:76:35)



npm ERR! Test failed.  See above for more details.
npm WARN This failure might be due to the use of legacy binary "node"
npm WARN For further explanations, please read
/usr/share/doc/nodejs/README.Debian

npm ERR! not ok code 0

pravi avatar Oct 28 '16 14:10 pravi

Hmm, they all pass locally for me. Anyway, will let @jonschlinkert weigh in

es128 avatar Oct 28 '16 14:10 es128

Let's revisit as soon as we get the glob-parent issue resolved. I think that will determine what we do here, and I think we're pretty close

jonschlinkert avatar Oct 28 '16 14:10 jonschlinkert

I think, this is a bug in is-glob (3.0 as well as 3.1),

var isGlob = require('is-glob');
console.log(isGlob('a/b/c/!foo'));

gives false.

pravi avatar Oct 28 '16 14:10 pravi

that's not a glob pattern. ! is a valid path character. To use negation inside of a path, you need to use a valid extglob, which would be a/b/c/!(foo)

edit: so this test looks like it's wrong.

jonschlinkert avatar Oct 28 '16 14:10 jonschlinkert

then its a fix in glob-base/test.js, right?

Current test,

globBase('a/b/c/!foo').should.eql({ base: 'a/b/c', isGlob: true, glob: '!foo' });

can I change this to,

globBase('a/b/c/!foo').should.eql({ base: 'a/b/c', isGlob: false});

pravi avatar Oct 28 '16 15:10 pravi

then its a fix in glob-base/test.js, right?

Exactly

can I change this to,

I think that's correct, except it looks like you would also need to add glob: '' (empty string). thanks

jonschlinkert avatar Oct 28 '16 15:10 jonschlinkert

But that seems not enough,

 {
         "base": "a/b/c"
      -  "glob": "!foo"
      +  "glob": ""
         "isGlob": false
       }

pravi avatar Oct 28 '16 15:10 pravi

can you clarify? Did that fail?

jonschlinkert avatar Oct 28 '16 16:10 jonschlinkert

Yes, it gave "glob": "!foo" instead of "glob": ""

pravi avatar Oct 28 '16 16:10 pravi

Hmm, even with the changes from glob-parent mentioned above? I thought you were saying that the following was passing:

globBase('a/b/c/!foo').should.eql({ base: 'a/b/c', isGlob: false});

Is that correct?

jonschlinkert avatar Oct 28 '16 16:10 jonschlinkert

I was asking if globBase('a/b/c/!foo').should.eql({ base: 'a/b/c', isGlob: false}); would be enough. I tried your suggestion and that failed.

$ npm test

> [email protected] test /home/pravi/forge/debian/git/pkg-javascript/glob-base
> mocha



  glob-base:
    ✓ typical globs:
    ✓ file extensions:
    1) negation pattern:
    ✓ extglobs:
    ✓ braces: no base:
    ✓ braces in filename:
    ✓ braces in dirname:
    ✓ regex character classes:
    ✓ qmarks:
    ✓ non-glob pattern:


  9 passing (49ms)
  1 failing

  1) glob-base: negation pattern::

      AssertionError: expected { base: 'a/b/c', glob: '!foo', isGlob: false } to equal { base: 'a/b/c', glob: '', isGlob: false } (at glob, A has '!foo' and B has '')
      + expected - actual

       {
         "base": "a/b/c"
      -  "glob": "!foo"
      +  "glob": ""
         "isGlob": false
       }

      at Assertion.fail (node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) [as eql] (node_modules/should/lib/assertion.js:81:17)
      at Context.<anonymous> (test.js:76:35)



npm ERR! Test failed.  See above for more details.
npm WARN This failure might be due to the use of legacy binary "node"
npm WARN For further explanations, please read
/usr/share/doc/nodejs/README.Debian

npm ERR! not ok code 0

pravi avatar Oct 28 '16 16:10 pravi

Hi in the meantime is-glob is at 4.0.0 hence PR #5 is also relevant.

What could be an estimated timeframe for fixing this ?

simevo avatar Dec 15 '17 09:12 simevo