analogjs / analog

The fullstack meta-framework for Angular. Powered by Vite and Nitro
https://analogjs.org
MIT License
2.48k stars 234 forks source link

Can't test with `it/describe.each` and `fakeAsync` #1115

Closed jahusa02 closed 4 weeks ago

jahusa02 commented 3 months ago

Please provide the environment you discovered this bug in.

https://stackblitz.com/~/github.com/jahusa02/analog-vitest

Which area/package is the issue in?

vitest-angular

Description

When a testfile contains it/describe.each and fakeAsync the tests errors out.

I described the error in this Issue https://github.com/analogjs/analog/issues/841#issuecomment-2110296265|

When I use the patched function (e.g. not importing it via import { it } from 'vitest'), the it.each tests will fail.

TypeError: Cannot read properties of undefined (reading 'withContext')
 ❯ ../../../packages/vite-plugin-angular/setup-vitest.ts:247:31
 ❯ src/lib/+state/effects/add-pending-contract/add-pending-contract.effects.spec.ts:98:6
     96|       { formStateValid: false, contractId: 'foo', satznummer: null },
     97|       { formStateValid: false, contractId: null, satznummer: 'bar' },
     98|     ])(
       |      ^
     99|       'should not post form values if ContractIds are %j',
    100|       ({ formStateValid, contractId, satznummer }) => {
 ❯ _ZoneDelegate.invoke ../../../node_modules/zone.js/fesm2015/zone.js:365:28
 ❯ ZoneImpl.run ../../../node_modules/zone.js/fesm2015/zone.js:111:43
 ❯ ../../../packages/vite-plugin-angular/setup-vitest.ts:41:21

When I importing it, the fakeAsync tests will fail with the mentioned Error: Expected to be running in 'ProxyZone', but it was not found., but the it.each will pass.

Please provide the exception or error you saw

Either

TypeError: Cannot read properties of undefined (reading 'withContext')

or

Error: Expected to be running in 'ProxyZone', but it was not found.

Other information

No response

I would be willing to submit a PR to fix this issue

gultyayev commented 1 month ago

I found a fix for it. Working on PR

jahusa02 commented 1 month ago

I found a fix for it. Working on PR

That would be awesome! Only thing that blocks my PR so we can finally use Vitest

gultyayev commented 1 month ago

In the meantime you could try a dirty patching of the node_modules in postinstall. The change is simple. I don't know how long it will take for the fix to appear in the release. Also, you could find potential missed cases.

brandonroberts commented 1 month ago

I planned to do a release today, but will wait on the PR to land. You can also test the beta after its merged

brandonroberts commented 1 month ago

1.6.0 contains the fixes

jahusa02 commented 1 month ago

Will test later, thanks!

jahusa02 commented 1 month ago

Still getting the Error:


 ❯ Function.assertPresent ../../../node_modules/zone.js/fesm2015/zone-testing.js:1973:19
 ❯ Function.setup ../../../../src/cdk/testing/testbed/task-state-zone-interceptor.ts:82:36
 ❯ new TestbedHarnessEnvironment ../../../../src/cdk/testing/testbed/testbed-harness-environment.ts:110:50
 ❯ TestbedHarnessEnvironment.createEnvironment ../../../../src/cdk/testing/testbed/testbed-harness-environment.ts:212:12
 ❯ TestbedHarnessEnvironment.createComponentHarness ../../../../src/cdk/testing/harness-environment.ts:154:33
 ❯ TestbedHarnessEnvironment._getQueryResultForElement ../../../../src/cdk/testing/harness-environment.ts:238:28
 ❯ ../../../../src/cdk/testing/harness-environment.ts:213:20
 ❯ ../../../../src/cdk/testing/harness-environment.ts:212:24
 ❯ ../../../../src/cdk/testing/change-detection.ts:190:49```
gultyayev commented 1 month ago

I assume it's the second case where you have the imports of utils? RN, it's not patched (by the plugin at all) and only globals should be used to work properly.

I've seen that we maybe could extend the Vitest's runners, but it looked more like using there functions from a specific place. E.g. import { describe, test ... } from 'analogjs-vitest-angular'. At least, that's what I grasped from their docs.

jahusa02 commented 1 month ago

Don't know why but npm decided to install the wrong version at first. (1.4.0 in lock file but 1.6.0 in package.json)

I still got the context error and had to import the globals to get rid of that. Will test later without them and a fresh install

gultyayev commented 1 month ago

Well. It works on my repo. However, describe.each is broken. By the same reason 🙃

Will need to make on more PR

gultyayev commented 1 month ago

Also, I cannot run Vitest freely. Only on a single thread. Otherwise I get segmentation fault on my macbook. @brandonroberts any ideas what it could be or how to fix? 🤔

brandonroberts commented 1 month ago

Also, I cannot run Vitest freely. Only on a single thread. Otherwise I get segmentation fault on my macbook. @brandonroberts any ideas what it could be or how to fix? 🤔

We use vmThreads by default, but you can change it to threads or forks and test with that

https://github.com/analogjs/analog/blob/beta/packages/vite-plugin-angular/src/lib/angular-vitest-plugin.ts#L14

https://vitest.dev/config/#pool

gultyayev commented 1 month ago

Could you elaborate on why those were chosen as default? The docs say that they are very prone to memory leaks. And even in my 500 tests project a 32 Gb RAM MacBook couldn't handle. Also, it left eventually a number of unclosed node processes.

brandonroberts commented 1 month ago

Could you elaborate on why those were chosen as default? The docs say that they are very prone to memory leaks. And even in my 500 tests project a 32 Gb RAM MacBook couldn't handle. Also, it left eventually a number of unclosed node processes.

vmThreads was chosen as the default to mimic the behavior with jest and jsdom

Here's some additional context from an issue I filed in the Vitest repo https://github.com/vitest-dev/vitest/issues/4685

jahusa02 commented 1 month ago

Thanks guys!

With the latest update (1.6.1) I was able to remove all imports of it and describe from my tests. Everything works now

brandonroberts commented 1 month ago

Great!