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

Collector fails with very large test counts (>> 100000) #5234

Closed
6 tasks done
DerYeger opened this issue Feb 19, 2024 · 3 comments · Fixed by #5235
Closed
6 tasks done

Collector fails with very large test counts (>> 100000) #5234

DerYeger opened this issue Feb 19, 2024 · 3 comments · Fixed by #5235
Assignees
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority)

Comments

@DerYeger
Copy link
Member

DerYeger commented Feb 19, 2024

Describe the bug

When a suite contains a huge number of tests (>> 100000), the collector's usage of the spread operator results in a "Maximum call stack size exceeded error".

Reproduction

Include a huge number of tests (e.g., using it.each).
See https://stackblitz.com/edit/vitest-dev-vitest-pf19qj?file=test%2Fsuite.test.ts.

System Info

System:
    OS: macOS 14.3.1
    CPU: (8) arm64 Apple M1
    Memory: 124.30 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/Library/Caches/fnm_multishells/20993_1708352067405/bin/node
    npm: 10.2.3 - ~/Library/Caches/fnm_multishells/20993_1708352067405/bin/npm
    pnpm: 8.15.1 - ~/Library/Caches/fnm_multishells/20993_1708352067405/bin/pnpm
    bun: 1.0.24 - ~/.bun/bin/bun
  Browsers:
    Chrome: 121.0.6167.184
    Safari: 17.3.1
  npmPackages:
    @vitest/coverage-v8: 1.2.2 => 1.2.2
    @vitest/ui: 1.2.2 => 1.2.2
    vitest: 1.2.2 => 1.2.2

Used Package Manager

pnpm

Validations

@DerYeger
Copy link
Member Author

See

tests.push(...getTests(task))

@hi-ogawa
Copy link
Contributor

Interesting find. It looks like this limitation is coming from runtime and people have especially have seen this when using Math.max(...) etc... https://stackoverflow.com/a/30834687

Just quick experiment on repl:

$ node
Welcome to Node.js v20.11.0.
Type ".help" for more information.
> [].push(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> Math.max(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> (() => {})(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> [...Array(1000000)]
[
  undefined, undefined, undefined, undefined, undefined, undefined,
  ...
]

I think it would nice if we can avoid it, but there might be a few more places doing push(...someArray) where the size of someArray is proportional to the number of tests.

@hi-ogawa hi-ogawa added p2-edge-case Bug, but has workaround or limited in scope (priority) and removed pending triage labels Feb 20, 2024
@DerYeger
Copy link
Member Author

Interesting find. It looks like this limitation is coming from runtime and people have especially have seen this when using Math.max(...) etc... https://stackoverflow.com/a/30834687

Just quick experiment on repl:

$ node
Welcome to Node.js v20.11.0.
Type ".help" for more information.
> [].push(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> Math.max(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> (() => {})(...Array(1000000))
Uncaught RangeError: Maximum call stack size exceeded
> [...Array(1000000)]
[
  undefined, undefined, undefined, undefined, undefined, undefined,
  ...
]

I think it would nice if we can avoid it, but there might be a few more places doing push(...someArray) where the size of someArray is proportional to the number of tests.

I've validated the changes by patching the @vitest/runner package and successfully running the entirety of my test project's test suite.
I did not add a new test case in my PR because I'm unsure if you'd like to have ~150.000 empty tests just for this.

@DerYeger DerYeger self-assigned this Feb 20, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants