[Bug]: running `nyc` in `reportCoverage()` fails on Windows with factory file associations settings

[pyryk]

Describe the bug

On Windows, with default Windows settings, generating the coverage report fails due to a Windows Script Host error.

When generating test coverage, the Storybook Test Runner runs the nyc command by executing the nyc.js file directly, rather than explicitly calling node with the file name as an argument. This works on systems that support the shebang directive, but AFAIK, on Windows, there is no shebang directive support. Therefore, Windows can only run the file based on the global file associations settings. Incidentally, the default file association for .js files is not node but the Windows Script Host. Unsurprisingly, the Windows Script Host is not able to interpret or run the nyc.js file.

Since the file associations on Windows are global to the system and prone to change and/or reset over time, I'd argue it would be beneficial to cross-platform compatibility if the test-runner coverage report function would not rely on those settings.

The previous test-runner version (0.17.0) tried to determine the running environment based on the package manager used. Is there a specific reason this approach was removed? As far as I understand, this approach also worked on Windows irrespective of file association settings. Another option would probably be explicitly calling node with the nyc.js file path as an argument (although I'm not sure if this approach would have some other downsides)

To Reproduce

Prerequisites: install node@20 on Windows 10 or 11

1. Reset Windows file associations to the default values (or set the file association for .js files to the Windows Script Host)
2. Add a minimal storybook example with a single storybook test
3. Run npm run test-storybook with coverage enabled
4. (Expected) a coverage report is generated (Actual) A Windows Script Host error popup is shown with message "Microsoft JScript compilation error: invalid character in file nyc.js on line 1, char 1" (paraphrased)

System

Storybook Environment Info:

  System:
    OS: Windows 11 10.0.22631
    CPU: (16) x64 AMD Ryzen 7 PRO 6850U with Radeon Graphics
  Binaries:
    Node: 20.9.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD <----- active
  Browsers:
    Edge: Chromium (123.0.2420.97)
  npmPackages:
    @storybook/addon-a11y: 8.0.10 => 8.0.10
    @storybook/addon-coverage: 1.0.3 => 1.0.3
    @storybook/addon-essentials: 8.0.10 => 8.0.10
    @storybook/addon-interactions: 8.0.10 => 8.0.10
    @storybook/addon-links: 8.0.10 => 8.0.10
    @storybook/addon-mdx-gfm: 8.0.10 => 8.0.10
    @storybook/react: 8.0.10 => 8.0.10
    @storybook/react-vite: 8.0.10 => 8.0.10
    @storybook/test: 8.0.10 => 8.0.10
    @storybook/test-runner: 0.18.0 => 0.18.0
    eslint-plugin-storybook: 0.8.0 => 0.8.0
    storybook: 8.0.10 => 8.0.10

Additional context

No response

[yannbf]

Hey @pyryk thanks for reporting this, and for being so detailed about it!

The previous implementation you mentioned actually never worked. As coverage generation runs in the process exit (which should be sync), the previous implementation, given it was asynchronous, would actually be lost and not execute. Your suggestion for running node is pretty good, I'm not sure about potential downsides either.

[yannbf]

@pyryk I made a canary release containing the fix. Would you please try it out?

@storybook/test-runner@0.18.1--canary.461.8fa90ff.0

[pyryk]

Thank you for such a quick response! I personally don't have a Windows machine I could test this on, but I asked a colleague to try the canary version. I'll let you know as soon as they've tried it.

[yannbf]

Hey @pyryk this is now released in version 0.18.1, please try it out!

[pyryk]

@yannbf the colleague just confirmed that the updated version works correctly on Windows even with default file associations.

Thank you for taking the time to fix this!

(I suppose it is now OK to close this, please reopen if there is a need for that)

[yannbf]

@yannbf the colleague just confirmed that the updated version works correctly on Windows even with default file associations.

Thank you for taking the time to fix this!

(I suppose it is now OK to close this, please reopen if there is a need for that)

Amazing to hear! Thank you so much for reporting!