search

karma-runner / karma-commonjs

A Karma plugin. Test CommonJS modules.
MIT License
73 stars 30 forks source link

Shouldn't run all modules by default #42

Open jamesshore opened 9 years ago

jamesshore commented 9 years ago

In commonjs_bridge.wrapper, karma-commonjs "requires" all files. This has the effect of running all files, which can cause odd behavior. The correct behavior is only to run files when a require statement is encountered.

I'm not sure what we should do here, but I think we need some sort of "entry point" configuration that says which files should be run.

pkozlowski-opensource commented 9 years ago

@jamesshore this is interesting. What kind of odd behaviour are you seeing in your tests? That is - what is the practical problem with this approach?

Somehow we should treat all tests being executed as "entry points" but I'm not sure how we could detect those...

jamesshore commented 9 years ago

I ran across when trying to npm install react@0.12.2 rather than vendor it. (See my React example for the vendor'd version.) Some code, such as node_modules/react/lib/warning.js, includes executable code in the body of the file, not just in exported functions. That code failed. This is also how I discovered #43. Files that were never actually require'd were being run.

Anyway, that got me thinking about correct behavior and I realized that running modules before they were require'd was actually incorrect.

andersekdahl commented 9 years ago

I encountered the same thing while making https://www.npmjs.com/package/karma-common-js, which is basically a fork of karma-commonjs.

The issues I had was that since modules were initialized in random order, subtle bugs appeared in the tests that were hard to figure out. When I added the ability to specify one or more entry points for the tests, all those issues were gone.

An example was a test which overrides window.setTimeout. When that wasn't started first, other modules could use setTimeout before it was mocked.