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

process.env.VITEST_WORKER_ID can be larger than maxThreads #1469

Closed
6 tasks done
flunderpero opened this issue Jun 12, 2022 · 13 comments · Fixed by #1473
Closed
6 tasks done

process.env.VITEST_WORKER_ID can be larger than maxThreads #1469

flunderpero opened this issue Jun 12, 2022 · 13 comments · Fixed by #1473

Comments

@flunderpero
Copy link

Describe the bug

We are migrating our Jest test code over to vitest. We use process.env.JEST_WORKER_ID to allocate a separate database for each worker/thread. With Jest that JEST_WORKER_ID is always between 1 and the maximum number of workers.

In vitest process.env.VITEST_WORKER_ID seems to have another meaning and is ever increasing. The documentation states that VITEST_WORKER_ID can be used to distinguish between threads. In my opinion this is not what it actually does.

Reproduction

Run any test-sutie and see that VITEST_WORKER_ID increases. In watch-mode it increases with each run.

System Info

System:
    OS: macOS 12.4
    CPU: (10) arm64 Apple M1 Max
    Memory: 1.36 GB / 64.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 3.1.0 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.10.0 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 102.0.5005.115
    Firefox: 100.0.2
    Safari: 15.5
  npmPackages:
    vitest: 0.14.2 => 0.14.2

Used Package Manager

yarn

Validations

@sheremet-va
Copy link
Member

We have a different implementation for workers then Jest. Each spec file fires a different worker (unlike Jest), so for each one you get a number. When watch is triggered, a new worker is created with a unique number. minThreads/maxThreads is passed down to tinypool, and is not used in the calculations.

@flunderpero
Copy link
Author

I totally get the idea behind this. Is there any way to get an id based on maxThreads so that we can use it in the way we used JEST_WORKER_ID or is this something you do not see any value in. (I certainly see a lot of value in it. :-) )

@sheremet-va
Copy link
Member

Can you describe how you see it? Is it just ID % maxThreads?

@flunderpero
Copy link
Author

Since VITEST_WORKER_ID starts at 1 it would have to be ((VITEST_WORKER_ID - 1) % maxThreads) + 1. For our purposes it would be enough to have maxThreads available somehow, so we can make the calculation ourselves. Perhaps it would be a good idea to make it clear in the documentation how VITEST_WORKER_ID works. vitest mimics the API of jest and others might equate VITEST_WORKER_ID with JEST_WORKER_ID (like I did).

@sheremet-va
Copy link
Member

#1473 introduces VITEST_POOL_ID with the logic you described

@sheremet-va
Copy link
Member

sheremet-va commented Jun 13, 2022

Beware that it will still change when editing file with watch (but not greater than maxThreads). Is it desirable behaviour?

@flunderpero
Copy link
Author

Wow, that was quick! It's ok for me that it may change. Thank you!

@flunderpero
Copy link
Author

I am afraid that the current implementation (and my suggestion here is wrong.

Imagine a use case where you have maxThreads=3 because you have 3 test databases that you want to use for 3 concurrently running tests. Now imagine the following run order:

VITEST_WORKER_ID -> VITEST_POOL_ID
Start 1                              1
Start 2                             2
Start 3                             3
End 2
Start 4                             1 !!!

In this scenario you have two threads with the same VITEST_POOL_ID and as a result two tests are using the same database. vitest would have to use some sort of bookkeeping to assign only "free" ids to VITEST_POOL_ID.

As a side note: The documentation already references VITEST_POOL_ID even though it is not released yet.

@sheremet-va
Copy link
Member

I don't think we have information about when test actually starts and ends. We just fire them all at the same time, and tinypool figures out the rest.

@flunderpero
Copy link
Author

Ah, I see. Mhhh, I will look into other possibilities to coordinate my test threads, but it's going to be way more complicated.

I would suggest to revert this change then, because I think it does not help anybody. Thank you again for your swift reaction, I like vitest a lot so far.

@sheremet-va
Copy link
Member

Ah, I see. Mhhh, I will look into other possibilities to coordinate my test threads, but it's going to be way more complicated.

I would suggest to revert this change then, because I think it does not help anybody. Thank you again for your swift reaction, I like vitest a lot so far.

We will try to fix this in a few weeks. See #1482

@sheremet-va
Copy link
Member

VITEST_POOL_ID should be correct in the next release.

@flunderpero
Copy link
Author

Yes, it is!

@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants