Source Map issues with Browserify and Webpack

[agilgur5]

Link to bug demonstration repository

agilgur5/physijs-webpack#coverage
Relevant files are browserify.js and webpack.js as well as their respective tests browserify.spec.js and webpack.spec.js which are all like 5-10 LoC each. package.json scripts and nyc.config.js for config.
(vendored files are physi.js, physijs_worker.js, and vendor/ammo.js, which are mostly ignorable)

For context, my library is an integration for bundlers, including out-of-the-box support for webpack and browserify, so my tests are literally to ensure that the integration itself works (and not that physijs works). An important aspect of this is WebWorker support for both bundlers.
This means that I have to test the bundled code, and not the source files themselves. The source code is all ES5, so no transpilation needed (though the test code is not necessarily). The code can be bundled for both bundlers with npm run build: test.

It seemed like nyc's source map support would handle this use case, but I ran into some issues with both bundlers.

Expected Behavior

1. I expect the output of npx nyc ava browserify.spec.js to be:

-----------------------------|----------|----------|----------|----------|-------------------|
File                         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------|----------|----------|----------|----------|-------------------|
All files                    |    100  |     100 |     100 |    100 |                   |
 browserify:                 |    100  |     100 |     100 |   100  |                   |
  browserify.js              |     100 |     100 |     100 |    100 |                   |
 ----------------------------|----------|----------|----------|----------|-------------------|

2. I expect the output of npx nyc ava webpack.spec.js to be:

-----------------------------|----------|----------|----------|----------|-------------------|
File                         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------|----------|----------|----------|----------|-------------------|
All files                    |    100  |     100 |     100 |    100 |                   |
 webpack:                    |    100  |     100 |     100 |   100  |                   |
  webpack.js                 |     100 |     100 |     100 |    100 |                   |
 ----------------------------|----------|----------|----------|----------|-------------------|

Observed Behavior

There are 2 different, but potentially related bugs, I'm encountering:

1. For browserify, I have source maps output to a separate .map file with exorcist, but none of the bundle itself or the source code ends up being output in the coverage. The output of npx nyc ava browserify.spec.js is:

----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |        0 |        0 |        0 |        0 |                   |
----------|----------|----------|----------|----------|-------------------|

I don't really know why the code in build/ and the source mapped code isn't included. The file I'm looking to be included without source maps is browserify.bundle.js and with source maps the files are browserify.js (and maybe physijs_worker.js and/or physi.js for further testing). Why are those not included?

2. For webpack, which runs virtually identical code (just a different bundler and worker-loader instead of webworkify), the output does cover webpack.bundle.js and the source mapped files, webpack.js, physi.js, and physijs_worker.js are shown in the coverage report. The output of npx nyc ava webpack.spec.js is:

-----------------------------|----------|----------|----------|----------|-------------------|
File                         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------|----------|----------|----------|----------|-------------------|
All files                    |    23.64 |     3.46 |     8.51 |    23.82 |                   |
 webpack:                    |    23.42 |     3.46 |     8.51 |    23.59 |                   |
  physi.js                   |    22.73 |     3.46 |     7.53 |     22.9 |... 1394,1395,1396 |
  physijs_worker.js          |      100 |      100 |      100 |      100 |                   |
  webpack.js                 |      100 |      100 |      100 |      100 |                   |
 webpack:/test-utils/webpack |      100 |      100 |      100 |      100 |                   |
  _export-to-window.js       |      100 |      100 |      100 |      100 |                   |
-----------------------------|----------|----------|----------|----------|-------------------|

The exclude is set to ignore test-utils/**, physi.js, and physijs_worker.js, but they still show up in the reported coverage through the source maps. (I'm also pretty sure the 100% coverage of physijs_worker.js is incorrect, but that's probably unrelated?). The code in test-utils that's imported directly (not source mapped) is correctly excluded/hidden, but the source mapped code is incorrectly shown. How do I exclude/hide these source mapped files?

There doesn't seem to be much documentation for testing source-mapped bundles. nyc seems to try to handle it automatically and there is the babel preset for babel instrumentation, but I'm not transpiling any code, just bundling it.
I also have both webpack and browserify set to output source maps to a separate .map file, but I'm not sure if that's necessary. The README says inline source maps are supported, but using cheap-eval-source-map for webpack causes the source maps to not work EDIT: it should be inline-cheap-source-map, woops.

Troubleshooting steps

• still occurring when I put cache: false in my nyc config

Environment Information

  System:
    OS: macOS High Sierra 10.13.6
    CPU: (4) x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
    Memory: 64.03 MB / 8.00 GB
  Binaries:
    Node: 8.4.0 - /usr/local/bin/node
    npm: 6.9.0 - /usr/local/bin/npm
  npmPackages:
    nyc: ^14.1.1 => 14.1.1

[agilgur5]

The issue hasn't had any recent activity because it's never been responded to....

[agilgur5]

Wanted to post an update here as I think I've narrowed down the issue(s) a bit in my latest trial-and-error experiment/investigation:

1. I believe that the no files output for the browserify version with nyc and ava and is probably the same problem I had with jest --coverage getting an infinite cycle error. I'm not totally sure why nyc/ava doesn't give any error but instead has empty output but they seem related at least.
   This issue seems like it's caused by istanbul-lib-instrument/ @babel/traverse based on jest's stacktrace (linked above). I think this is because it has trouble parsing ammo.js, a ~1.2MB Emscripten port. I had some similar (but not exactly the same) issues when babel was running over the ammo code (20+ second runs and garbage collection errors that would eventually crash). When I used babel-plugin-istanbul to manually instrument the bundle and ignored ammo from babel processing, it seemed to instrument and run fine.

2. Bundling plus istanbul coverage (with or without source maps) seems to have issues and there seems to be no way to exclude some files. Like I can't exclude the webpack bootstrap or universalModuleDefinition (which is not source-mapped to anything). Trying to exclude it with like webpack.bundle.js:/webpack etc just doesn't work.
   Trying to exclude actual source code, like say, test/utils/, doesn't work either, neither with trying to exclude the bundled "files" (webpack.bundle.js:/Physijs/test/utils/) nor the source file (/test/utils/). It similarly does not work when source maps are included (which basically just changed it to say physijs-webpack/test/utils/ in the output instead).
   It seems like the only way to get excludes to work with a bundle is to manually instrument it.

3. I have no idea how to get coverage for a real Web Worker (i.e. one that runs in a separate thread, not a fake one that runs in the same thread). agilgur5/physijs-webpack#cypress-coverage is able to run everything correctly in Cypress, and with manual instrumentation files can be properly excluded from coverage reports. But physijs_worker.js doesn't appear in the coverage reports at all 😞 😕 😖

[agilgur5]

If so, what is blocking it?

It's never been responded to in over a year

Is there anything you can do to help move it forward?

I've debugged it heavily and added lots of details and links and the repo is open-source and quite tiny. The only other thing I can do is dive into the nyc code but I don't even know where to start and have not been given any guidance.

[coreyfarrell]

I noticed your are using nyc 14, there were some source-map fixes which went into nyc 15 (particularly related to bundled code), please give this a try to see if it improves the ability to ignore parts of the bundle. You should be able to tell istanbul to exclude src/some-file.js. This would still produce coverage counters inside the bundle but when nyc report sees that exclude it would drop the coverage associated by source-maps to src/some-file.js from the report. This was done in nyc 15 only as it is a breaking change - I've gotten bug reports for 15 due to people not realizing the change effected their project. Note upgrading from nyc 14 to 15 may require upgrading 3rd party components to ensure the only istanbul-* modules are those versions installed by nyc 15.

With regard to browser web workers this is completely out of scope for nyc as nyc does not run in the browser. If you create a simple file.js then run nyc instrument --compact=false file.js you will see the code we generate. The code generated by istanbul initializes a global __coverage__ variable. nyc has hooks into node.js which intercept process exit and write JSON.stringify(global.__coverage__) to a file in .nyc_output. When you run code in the browser it is the responsibility of the browser automation library to handle this upon each JS global context exit. For most code this means the browser automation library will hook page navigation and grab the variable. I'm not sure if they can do the same for web worker threads (which each have their own isolated JS global context). This would be a question for cypress and ultimately it would require that the browser provide necessary hooks for cypress to detect "this web worker is about to exit".

Sorry this issue has sat for so long without response, istanbul maintainers are spread very thin and integration issues require an additional level of knowledge. I've blocked stale bot from touching this issue in the future. My hope is that someone in the community with knowledge of webpack / browserify can provide some insights. Please understand nyc is designed to test coverage in node.js. nyc does not directly support any other platform, though the instrumented code it generates does try to support running on as many JS platforms as possible. Please be aware source-map handling is probably the most difficult part of istanbul, it's a minefield. That's not completely a reflection of our code but source-maps are just difficult.

[JessicaSachs]

Hey @coreyfarrell,

Thanks for your detailed writeup. This is gonna be in response to WebWorkers and I just realized that this comment maybe belongs in a diff repo or issue, but here we go...

With regard to browser web workers this is completely out of scope for nyc as nyc does not run in the browser.

So, in a browser-testing setup, Istanbul's transpiled code is running in the browser, while coverage results are sent back over the wire to Node so that NYC can consume and process the results. NYC is still node-side, but because it wraps + kicks off Istanbul, I need to be able to pass in certain options to a given instrumenter. Can you help me figure out how to configure Istanbul's options through NYC?

I need to either:

1. Skip transpilation of files matching a certain pattern (e.g. the web workers) and lose coverage (that's fine)
2. Fix transpilation by making Istanbul's instrument options to work for WebWorkers (e.g. using globalThis instead of this or window).

https://github.com/istanbuljs/istanbuljs/blob/2b747cfc636a6e9b991d30fe5d25fec3667ff151/packages/istanbul-lib-instrument/src/instrumenter.js#L100-L102

So if I hardcode Istanbul's coverageGlobalScope to use globalThis instead of this (window), then my browser tests run fine and coverage works for WebWorkers... but I can only do it with patch-package or pnpm patch because I don't know how to set coverageGlobalScope 😅 .

Workaround patch here for istanbul-lib-instrument:

diff --git a/src/visitor.js b/src/visitor.js
index 46c71290d05c04c8f07a4421e9aab6cdbd74ec6e..0cb9f48c9f1dcd01f261e5fb62bc293ff68415ed 100644
--- a/src/visitor.js
+++ b/src/visitor.js
@@ -749,7 +749,8 @@ function programVisitor(types, sourceFilePath = 'unknown.js', opts = {}) {
     const T = types;
     opts = {
         ...defaults.instrumentVisitor,
-        ...opts
+        ...opts,
+        coverageGlobalScope: 'globalThis'
     };
     const visitState = new VisitState(
         types,

How can I pass through coverageGlobalScope from NYC => istanbul-lib-instrument?