vitest icon indicating copy to clipboard operation
vitest copied to clipboard

Run all async test concurrently without threads and isolate

Open NikolayMakhonin opened this issue 3 years ago • 14 comments

Clear and concise description of the problem

I need it for parallel playwright tests using browser pool instead run and wait new browser for each test. And so I need:

  1. disable isolate to reuse browser pool instance for all tests
  2. run all test files in parallel

If I will use threads then:

  1. I can't reuse single browser pool instance for all tests
  2. tests will run slower

Suggested solution

Add new options for run all test files in parallel

Alternative

  1. use single test for all my playwright tests
  2. use this hack:
import {globby} from 'globby'

describe.concurrent('all', async function () {
  const testFiles = await globby('src/**/*.test*')
  await Promise.all(testFiles.map(async testFile => {
    await import(testFile)
  }))
})

but watch doesn't work with dynamic imports and I can't specify custom watch files

Additional context

No response

Validations

NikolayMakhonin avatar Jul 12 '22 03:07 NikolayMakhonin

Is this a duplicate https://github.com/vitest-dev/vitest/issues/1533?

but watch doesn't work with dynamic imports and I can't specify custom watch files

You can specify watchRerunTriggers to rerun all tests on change.

sheremet-va avatar Jul 12 '22 05:07 sheremet-va

Is this a duplicate #1533?

I think no, because I want to run all files in parallel, but #1533 doesn't solve this task. I need something like describe.concurrent for all files, something like this:

all-tests.js

describe.concurrent('all', async function () {
  const testFiles = await globby('src/**/*.test*')
  await Promise.all(testFiles.map(async testFile => {
    await import(testFile)
  }))
})

test1.test.js

it ('test1', async function() {
   // async browser test 1
})

this code is working, but without watch feature - i seems like the watch can't work with the dynamic imports

NikolayMakhonin avatar Jul 12 '22 05:07 NikolayMakhonin

Files are already running in parallel.

sheremet-va avatar Jul 12 '22 05:07 sheremet-va

Files are already running in parallel.

I prepared a repo for reproduce this issue.

I added specific timeouts to 3 tests, so the comments should be displayed in order in time, not in launch order. You can see that the tests are run one by one and not in parallel. Probably because threads is disabled, but I need to disable threads for use one browser pool instance for all tests.

I expect this in the console:

stdout | src/test.js > all > test3
test2_1: order=1

stdout | src/test.js > all > test3
test1_1: order=2

stdout | src/test.js > all > test3
test3_1: order=3

stdout | src/test.js > all > test3
test2_2: order=4

stdout | unknown test
test1_2: order=5

stdout | unknown test    
test3_2: order=6         

but I got this:

stdout | src/test3.test.js > test3
test3_1: order=3

stdout | src/test3.test.js > test3
test3_2: order=6

stdout | src/test1.test.js > test1
test1_1: order=2

stdout | src/test1.test.js > test1
test1_2: order=5

stdout | src/test2.test.js > test2
test2_1: order=1

stdout | src/test2.test.js > test2
test2_2: order=4                  

But if you run my hack (src/test.js) all test will run in parallel.

NikolayMakhonin avatar Jul 12 '22 06:07 NikolayMakhonin

Yes, this code is running your tests:

https://github.com/vitest-dev/vitest/blob/e910f81097f9a8fbeb27db4c6015fb171faaacf4/packages/vitest/src/runtime/entry.ts#L39

It's not obvious how we can run it in parallel with our threads, since cache and mocking are depending on it. In your example you have only one actual test file, the ones that you import are considered subtests of this spec.

sheremet-va avatar Jul 12 '22 06:07 sheremet-va

These changes in vitest is worked for me. All tests running in parallel, but I think that this brakes other features. Perhaps the project architecture doesn't allow to run test in parallel in single thread.

NikolayMakhonin avatar Jul 12 '22 08:07 NikolayMakhonin

But maybe you could at least add the ability to the watch feature to handle dynamic imports?

NikolayMakhonin avatar Jul 12 '22 08:07 NikolayMakhonin

These changes in vitest is worked for me. All tests running in parallel, but I think that this brakes other features. Perhaps the project architecture doesn't allow to run test in parallel in single thread.

With this change imports inside tests are not reevaluated for each test, meaning setting variable of a some imported module:

// mod.ts
let test = 1
export const getTest = () => test
export const setTest = (t) => test = 1
// spec.ts
import { setTest } from './mod.ts'
setTest(2)
// spec2.ts
import { getTest } from './mod.ts'
getTest() // sometimes 1, sometimes 2

will change the variable for all modules.

Also vi.mock inside one spec file will affect other spec files.

sheremet-va avatar Jul 12 '22 08:07 sheremet-va

But maybe you could at least add the ability to the watch feature to handle dynamic imports?

Watch relies on Vite module graph, which doesn't support dynamic imports. You can use import.meta.glob.

sheremet-va avatar Jul 12 '22 08:07 sheremet-va

will change the variable for all modules

This is what I want - fully disable the test isolation. I certainly won't do it as in your example, I'll use the common file with browser pool singleton for all tests.

NikolayMakhonin avatar Jul 12 '22 08:07 NikolayMakhonin

Yes, the import.meta.glob is worked, i will leave a working example here. This suits me as a temporary solution. Thanks.

NikolayMakhonin avatar Jul 12 '22 08:07 NikolayMakhonin

I still returned to this option, because this is simpler, retains all vitest's watch functions and removes the test isolation. I made the @flemist/vitest module with the concurrentFiles option.

NikolayMakhonin avatar Jul 12 '22 13:07 NikolayMakhonin

I can see from the code that this is indeed an architectural problem that is not so easy to fix without breaking other functions. It turns out that in order to run asynchronous tests in parallel, it is necessary to run each test file in a separate worker. Considering that tests are asynchronous, and asynchronous requests in browser tests are slow, workers will be idle and will not do the work for which they are intended - to load the processor to the maximum. Or the developer will need to create dozens of workers, which can cause memory overflow or reduce performance. In my opinion, the speed of test execution is more important than their isolation. Isolation is needed for "protection from a fool" or for some rare cases, because in my practice it was never needed.

NikolayMakhonin avatar Jul 13 '22 03:07 NikolayMakhonin

This is not a "problem". We have people asking to isolate even each test, so this is just one person's word agains the other.

We need to run test one ofter another, because we depend on a global variable. If we can pass down all dependencies for test, we can run them in parallel.

sheremet-va avatar Jul 13 '22 04:07 sheremet-va

We don't think this is worth pursuing.

sheremet-va avatar Oct 04 '23 09:10 sheremet-va