Remove fast-glob dependency #162

MicahZoltu · 2020-07-26T07:23:03Z

Jasmine currently has two dependencies, jasmine-core and fast-glob. jasmine-core has no transitive dependencies, but fast-glob has ~93 transitive dependencies. I'm not sure what exactly fast-glob is being used for, but it certainly sounds like a library that is not a critical piece of Jasmine, and certainly not something that needs 93 dependencies to function.

Consider looking for alternative implementations to use, or inline whatever functionality it is you need from fast-glob. If it weren't for that one dependency, Jasmine would be an incredibly low dependency testing framework, which is exactly what I am looking for (and why I used to use Jasmine).

An significant iterative improvement here would be to just update fast-glob to latest, as it has greatly reduced its dependency set between version 2.2.7 and 3.2.4 (from 93 to 16).

Edit: Looks like my estimate was a bit off, in reality it appears to have 121 transitive dependencies, all from fast-glob:

npm install jasmine
npm WARN deprecated [email protected]: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated [email protected]: https://github.com/lydell/resolve-url#deprecated
added 121 packages from 101 contributors and audited 146 packages in 3.215s

The text was updated successfully, but these errors were encountered:

- See #162

slackersoft · 2020-07-28T02:15:16Z

Jasmine uses file globbing to allow users to specify wildcards in their list of spec files to run. Previous to this release we used the glob package, but ended up picking up fast-glob as part of #153. I've updated the dependency to use the most recent fast-glob, but we won't likely make a new release just for this.

Thanks for reporting the issue on transitive dependencies. We try to keep Jasmine's dependency graph small, but sometimes things sneak in or there just isn't a good way around them.

apla · 2020-08-15T07:09:25Z

http://npm.broofa.com/?q=jasmine

slackersoft · 2020-09-03T00:21:49Z

I've updated the version of fast-glob for the next release, but that doesn't quite seem like enough of an impetus to release a 3.6.2 of Jasmine, since that's currently the only change since the 3.6.1 release.

Globbing files has been part of Jasmine for some time now, so we don't want to just remove it. I'd be happy to have a discussion on which library we can use to do the globbing (ideally something with support and enough users), or if the update to the more recent version of fast-glob is good enough.

vfcp · 2020-09-04T10:12:23Z

Since you opened the debate about the library, I'm curious why the switch from glob to fast-glob in #153 ?

Looking at the dependencies in some of my projects, I can see some other popular packages like node-sass, rollup, rimraf, node-gyp all use glob. Actually, according to npmjs, glob has 18851 dependents vs fast-glob's 1117.

While I totally agree that popularity doesn't mean better, I believe consistent glob behavior with other tools commonly used with jasmine would be more beneficial than a (potential) few milliseconds performance boost.

But then again, I have no data to compare between glob and fast-glob in a large project and decisions should be supported by facts, not opinions :)

MicahZoltu · 2020-09-04T11:17:50Z

Data for comparison:
glob has 10 dependencies.
fast-glob (latest) has 16 dependencies.

vfcp · 2020-09-04T13:21:13Z

Becoming aware of #153 , I've decided to give --worker-count a try. There seems to be a lot of issues with it. To be sure the issues were not related with my specs, I've cloned jasmine-npm repo and modified package.json test script from "test": "./node_modules/.bin/grunt && ./bin/jasmine.js" to "test": "./node_modules/.bin/grunt && ./bin/jasmine.js --worker-count=2"

vfcp@Valters-MacBook jasmine-npm % npm test         

> [email protected] test /Users/vfcp/Projects/jasmine-npm
> grunt && ./bin/jasmine.js --worker-count=2

Running "jshint:all" (jshint) task
>> 28 files lint free.

Running "specs" task
Randomized with seed 55099
Started
.................................................................................................................................


129 specs, 0 failures
Finished in 0.293 seconds
Randomized with seed 55099 (jasmine --random=true --seed=55099)

Done.
Randomized with seed 26796
Started
..............................................................................F.FFF.F.F.FF..FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:4037
          throw error;
          ^

TypeError: Cannot read property 'spies' of undefined
    at currentSpies (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1840:56)
    at SpyRegistry.clearSpies (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:8130:19)
    at clearResourcesForRunnable (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1387:19)
    at QueueRunner.onComplete (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1733:13)
    at Immediate.<anonymous> (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7120:12)
    at processImmediate (internal/timers.js:458:21) {
  jasmineMessage: "Uncaught exception: TypeError: Cannot read property 'spies' of undefined"
}
FFFFFFFFFFFFF./Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:4037
          throw error;
          ^

TypeError: Cannot read property 'spies' of undefined
    at currentSpies (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1840:56)
    at SpyRegistry.clearSpies (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:8130:19)
    at clearResourcesForRunnable (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1387:19)
    at QueueRunner.onComplete (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1733:13)
    at Immediate.<anonymous> (/Users/vfcp/Projects/jasmine-npm/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:7120:12)
    at processImmediate (internal/timers.js:458:21) {
  jasmineMessage: "Uncaught exception: TypeError: Cannot read property 'spies' of undefined"
}


Failures:
1) manager reporter callback 'specStarted' should forward the payload directly
  Message:
    Error: <spyOn> : fork() method does not exist
    Usage: spyOn(<object>, <methodName>)
  Stack:
    Error: <spyOn> : fork() method does not exist
    Usage: spyOn(<object>, <methodName>)
        at <Jasmine>
        at UserContext.<anonymous> (/Users/vfcp/Projects/jasmine-npm/spec/manager_spec.js:10:5)
        at <Jasmine>
  Message:
    TypeError: cluster.fork is not a function
  Stack:
    TypeError: cluster.fork is not a function
        at exports (/Users/vfcp/Projects/jasmine-npm/lib/manager.js:29:13)
        at UserContext.<anonymous> (/Users/vfcp/Projects/jasmine-npm/spec/manager_spec.js:58:7)
        at <Jasmine>
        at processImmediate (internal/timers.js:458:21)

2) manager reporter callback 'specDone' should forward the payload directly
  Message:
    Error: <spyOn> : fork() method does not exist
    Usage: spyOn(<object>, <methodName>)
  Stack:
    Error: <spyOn> : fork() method does not exist
    Usage: spyOn(<object>, <methodName>)
        at <Jasmine>
        at UserContext.<anonymous> (/Users/vfcp/Projects/jasmine-npm/spec/manager_spec.js:10:5)
        at <Jasmine>
  Message:
    TypeError: cluster.fork is not a function
  Stack:
    TypeError: cluster.fork is not a function
        at exports (/Users/vfcp/Projects/jasmine-npm/lib/manager.js:29:13)
        at UserContext.<anonymous> (/Users/vfcp/Projects/jasmine-npm/spec/manager_spec.js:71:7)
        at <Jasmine>
        at processImmediate (internal/timers.js:458:21)

... up to 92)

Did I misunderstood the --worker-count option?

apla · 2020-09-04T13:44:36Z

Data for comparison:
glob has 10 dependencies.
fast-glob (latest) has 16 dependencies.

And now it's much better, shaving off 76 deps. Surprisingly, micromatch@3 package have 81 deps itself despite it's name.

slackersoft · 2020-09-04T16:45:15Z

Honestly, I didn't look too deeply at the details of the switch from glob to fast-glob in the multi-threading PR, other than at some of the benchmarks. I don't have a strong opinion either way on which library Jasmine uses to do file globbing. The main argument I'm hearing in favor of switching back to glob is that any given project is more likely to already have that be a transitive dependency than fast-glob.

I'm inclined to leave this decision a bit up to a vote, since the community is ultimately who gets stuck with the larger dependency tree.

👍 - Switch back to glob
👎 - Stick with (updated) fast-glob

- This functionality doesn't appear to have ever worked. It looks like there are some changes necessary to jasmine-core to be able to support multiple workers, but the original submitter didn't actually try this out - See #153 - Fixes #163 - Fixes #165 - See also discussion in #162

slackersoft pushed a commit that referenced this issue Jul 28, 2020

Update dependencies

e6e7ed8

- See #162

vfcp mentioned this issue Sep 4, 2020

#153 --worker-count issues #163

Closed

slackersoft closed this as completed in 5e153dc Sep 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove fast-glob dependency #162

Remove fast-glob dependency #162

MicahZoltu commented Jul 26, 2020 •

edited

Loading

slackersoft commented Jul 28, 2020

apla commented Aug 15, 2020

slackersoft commented Sep 3, 2020

vfcp commented Sep 4, 2020

MicahZoltu commented Sep 4, 2020

vfcp commented Sep 4, 2020 •

edited

Loading

apla commented Sep 4, 2020

slackersoft commented Sep 4, 2020

Remove fast-glob dependency #162

Remove fast-glob dependency #162

Comments

MicahZoltu commented Jul 26, 2020 • edited Loading

slackersoft commented Jul 28, 2020

apla commented Aug 15, 2020

slackersoft commented Sep 3, 2020

vfcp commented Sep 4, 2020

MicahZoltu commented Sep 4, 2020

vfcp commented Sep 4, 2020 • edited Loading

apla commented Sep 4, 2020

slackersoft commented Sep 4, 2020

MicahZoltu commented Jul 26, 2020 •

edited

Loading

vfcp commented Sep 4, 2020 •

edited

Loading