Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: Concurrency issue on some devices with insufficient CPU access #1579

Open
1 task done
mitsuki31 opened this issue Aug 22, 2024 · 0 comments
Open
1 task done

Comments

@mitsuki31
Copy link

Link to bug demonstration repository

  1. mitsuki31/ytmp3-js
    • Run npm i && npm run coverage. The command will fail if your environment lacks sufficient permissions to access CPU information.
  2. mitsuki31/nyc (main branch)

Expected Behavior

The concurrency value should default to 1 if the os.cpus() function fails to retrieve the CPU count, rather than setting it to 0, which causes the tests to fail.

For example, this line:

nyc/index.js

Line 241 in ae657b6

const concurrency = output ? os.cpus().length : 1

For some environments, we might need to give os.cpus().length a fallback value to prevent it returns 0.

- const concurrency = output ? os.cpus().length : 1
+ const concurrency = output ? (os.cpus().length || 1) : 1

Observed Behavior

  • From my project:
    Expected `concurrency` to be a number from 1 and up, got `0` (number)
  • From the forked NYC repo (in main branch):
     FAIL  test/add-all-files.js 4 failed of 5 22s
     ✖ outputs an empty coverage report for all files that are not excluded >   node_modules/p-map/index.
       Expected `concurrency` to be a number from 1 and up, got `0` (number)    js:18:10
     ✖ outputs an empty coverage report for multiple configured extensions >    node_modules/p-map/index.
       Expected `concurrency` to be a number from 1 and up, got `0` (number)    js:18:10
     ✖ transpiles .js files added via addAllFiles > Expected `concurrency` to benode_modules/p-map/index.
        a number from 1 and up, got `0` (number)                                js:18:10
     ✖ transpiles non-.js files added via addAllFiles > Expected `concurrency`  node_modules/p-map/index.
       to be a number from 1 and up, got `0` (number)                           js:18:10

Note

The error messages were sourced from p-map module.

Troubleshooting steps

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

I have identified a solution in my forked repository, specifically in the fix/fix-concurrency-issue branch.
However, there may be a minor issue with the coverage threshold, which is not meeting the 100% requirement. I'm unsure where to create a test case to address this.

Consider to check the codebase comparison and the CI result.

Comparison: main...mitsuki31:nyc:fix/fix-concurrency-issue
CI Result: https://github.com/mitsuki31/nyc/actions/runs/10512824982

Environment Information

  System:
    OS: android
    CPU: Unknown
    Memory: 1.58 GB / 3.89 GB
  Binaries:
    Node: 20.15.1 - /data/data/com.termux/files/usr/bin/node
    npm: 10.7.0 - /data/data/com.termux/files/usr/bin/npm
  npmPackages:
    istanbul-lib-coverage: ^3.0.0 => 3.2.2
    istanbul-lib-hook: ^3.0.0 => 3.0.0
    istanbul-lib-instrument: ^6.0.2 => 6.0.2
    istanbul-lib-processinfo: ^2.0.2 => 2.0.3
    istanbul-lib-report: ^3.0.0 => 3.0.1
    istanbul-lib-source-maps: ^4.0.0 => 4.0.1
    istanbul-reports: ^3.0.2 => 3.1.7
    source-map-support: ^0.5.16 => 0.5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant