quasarframework / quasar-testing

Testing Harness App Extensions for the Quasar Framework 2.0+
https://testing.quasar.dev
MIT License
179 stars 65 forks source link

Vitest fails with Qdialog/Qinput errors #292

Closed IAmTheMilkManMyMilkIsDelicious closed 1 year ago

IAmTheMilkManMyMilkIsDelicious commented 1 year ago

While running my test suite in I will sometimes run into this issue somewhere in the middle of the run, the error fully crashes my test suite so it doesn't even finish the run, as well as that it won't say which test it comes from.

I think the issue is similar to the issue with QLayouts that required injections, this time affecting QDialog/QInput: https://github.com/quasarframework/quasar-testing/issues/269

Software version

OS: Linux (Docker) Node: 18.12.0 NPM: 8.19.1 @quasar/quasar-app-extension-testing-unit-vitest: 0.1.2 env: 'happy-dom'

What did you get as the error?

node:internal/event_target:1006
  process.nextTick(() => { throw err; });
                           ^
ReferenceError [Error]: document is not defined
    at Array.<anonymous> (/builds/frontend/node_modules/quasar/src/components/input/QInput.js:195:20)
    at Module.removeFocusWaitFlag (/builds/frontend/node_modules/quasar/src/utils/private/focus-manager.js:19:30)
    at hidePortal (/builds/frontend/node_modules/quasar/src/composables/private/use-portal.js:83:27)
    at Timeout._onTimeout (/builds/frontend/node_modules/quasar/src/components/dialog/QDialog.js:261:9)
    at listOnTimeout (node:internal/timers:564:17)
    at process.processTimers (node:internal/timers:507:7)
Emitted 'error' event on Tinypool instance at:
    at EventEmitterReferencingAsyncResource.runInAsyncScope (node:async_hooks:203:9)
    at Tinypool.emit (file:///builds/frontend/node_modules/tinypool/dist/esm/index.js:60:31)
    at Worker.<anonymous> (file:///builds/frontend/node_modules/tinypool/dist/esm/index.js:566:30)
    at Worker.emit (node:events:513:28)
    at [kOnErrorMessage] (node:internal/worker:290:10)
    at [kOnMessage] (node:internal/worker:301:37)
    at MessagePort.<anonymous> (node:internal/worker:202:57)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:731:20)
    at exports.emitMessage (node:internal/per_context/messageport:23:28)

Node.js v18.12.0
error Command failed with exit code 1.

What were you expecting?

No error is thrown, ending the test

What steps did you take, to get the error?

Ran a test suite using vitest, testing components that contain QDialogs and QInputs. The error is more likely on slower hardware.

IlCallo commented 1 year ago

Seems like a race condition to me, something like a timeout set by a Dialog or QInput which leaks out of the test Can you check out if you hit the same problem into Node v16? Or if there has been similar reports on Vitest directly It doesn't seem a thing we can debug properly without a repro, and I'd say it's not in the scope of this package

IAmTheMilkManMyMilkIsDelicious commented 1 year ago

Just tried locally with node v16.15.0 and reproduced it again.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
 ❯ Array.<anonymous> node_modules/quasar/src/components/input/QInput.js:186:20
ReferenceError: document is not defined
    184|     function focus () {
    185|       addFocusFn(() => {
    186|         const el = document.activeElement
       |                    ^
    187|         if (
    188|           inputRef.value !== null
 ❯ Module.removeFocusWaitFlag node_modules/quasar/src/utils/private/focus-manager.js:18:30
 ❯ hidePortal node_modules/quasar/src/composables/private/use-portal.js:78:4
 ❯ Timeout._onTimeout node_modules/quasar/src/components/dialog/QDialog.js:247:9
 ❯ listOnTimeout node:internal/timers:559:17
 ❯ processTimers node:internal/timers:502:7
IlCallo commented 1 year ago

What I see there is that the error always originate from a Dialog timeout, that's why I think it's a race-condition due to a poor cleanup of existing timers. I think Jest just ignores those kind of things or automatically cleans it up

It may be worth testing if it has been introduced in a recent Quasar release. If it is, we probably need to fix it into Quasar core

IAmTheMilkManMyMilkIsDelicious commented 1 year ago

Turns out I was mistakenly using quasar 2.7.7. I upgraded to 2.10.1 and the issue changed: It became a few issues.

Firstly:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
 ❯ QuerySelector.querySelector node_modules/happy-dom/lib/query-selector/QuerySelector.js:44:34
 ❯ Proxy.querySelector node_modules/happy-dom/lib/nodes/element/Element.js:675:40
 ❯ node_modules/quasar/src/components/dialog/QDialog.js:242:40
TypeError: Cannot read properties of undefined (reading 'includes')
    240|         }
    241| 
    242|         node = (selector !== '' ? node.querySelector(selector) : null)
       |                                        ^
    243|           || node.querySelector('[autofocus][tabindex], [data-autofocus][tabindex]')
    244|           || node.querySelector('[autofocus] [tabindex], [data-autofocus] [tabindex]')
 ❯ Module.addFocusFn node_modules/quasar/src/utils/private/focus-manager.js:25:5
 ❯ focus node_modules/quasar/src/components/dialog/QDialog.js:235:6
 ❯ Module.<anonymous> node_modules/quasar/src/composables/private/use-tick.js:32:42

However I can fix that by changing quasar/src/components/dialog/QDialog.js:242 from:

         node = (selector !== '' ? node.querySelector(selector) : null)

to

        node = ((selector !== '' && selector ) ? node.querySelector(selector) : null)

I think this issue has come from a recent change here


Secondly:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
ReferenceError: document is not defined
 ❯ Array.<anonymous> node_modules/quasar/src/components/dialog/QDialog.js:238:44
    236|         let node = innerRef.value
    237| 
    238|         if (node === null || node.contains(document.activeElement) === true) {
       |                                            ^
    239|           return
    240|         }
 ❯ Module.removeFocusWaitFlag node_modules/quasar/src/utils/private/focus-manager.js:18:30
 ❯ showPortal node_modules/quasar/src/composables/private/use-portal.js:53:6
 ❯ Timeout._onTimeout node_modules/quasar/src/components/dialog/QDialog.js:208:9
 ❯ listOnTimeout node:internal/timers:564:17
 ❯ process.processTimers node:internal/timers:507:7

I can fix that by changing quasar/src/components/dialog/QDialog.js:238 from:

        if (node === null || node.contains(document.activeElement) === true) {

to

        if (node === null || typeof document === 'undefined' || node.contains(document.activeElement) === true) {

After making those two changes and running my tests a few dozen times, I can no longer reproduce the original issue using 2.10.1. I can make a new PR to the quasar repo with these two line changes, but I don't know enough about the code to know if my changes will break things for other users.

IlCallo commented 1 year ago

They both seem to be related to a mismatch between browser and happy-dom code implementation and execution We currently ship with v5 of happy-dom, but I just noticed v7 has been released already

Try upgrading happy-dom to latest and see if the problems persist, also check out into happy-dom repo if similar issues has been reported

IlCallo commented 1 year ago

We definitely need to bump both Vitest and happy dom dependencies, please try to bump them manually in you project and see if it helps

IAmTheMilkManMyMilkIsDelicious commented 1 year ago

Okay I'm now using these settings:

Node: 18.12.0
NPM: 8.19.1
"@quasar/quasar-app-extension-testing-unit-vitest": "0.1.2"
"quasar": "2.10.1"
"vitest": "0.24.5"
"happy-dom": "7.6.6"

It didn't make any difference unfortunately. The first issue: Cannot read properties of undefined (reading 'includes') happens every time and the second issue: document is not defined happens some of the time. The errors are identical to the ones above.

IlCallo commented 1 year ago

Ok, the first one seems legitimate and reproducible, please open an issue with a repro on Quasar main repo, and related PR to solve it The second one seems a race condition due to poor cleanup either by Quasar or Vitest, open an issue on both repos about that, the root cause will probably be harder to pinpoint

IAmTheMilkManMyMilkIsDelicious commented 1 year ago

I can reliably reproduce the first issue now. I discovered the issue is thrown when using vitest fake timers.

ExampleComponent.vue

<template>
  <q-dialog v-model="showDialog"/>
</template>
<script setup lang="ts">
import { ref } from 'vue';
const showDialog = ref(false)
</script>

ExampleComponent.test.ts

import { mount } from '@vue/test-utils';
import { describe, it, vi } from 'vitest';
import ExampleComponent from 'src/components/ExampleComponent.vue';
import { nextTick } from 'vue';
import { installQuasar } from '@quasar/quasar-app-extension-testing-unit-vitest';

installQuasar();

describe('Test', () => {

  it('should not throw errors when using faketimers', async () => {
    const  wrapper = mount(ExampleComponent);
    vi.useFakeTimers();
    wrapper.vm.showDialog = true;
    await nextTick();
    vi.runAllTimers();
  });

});

Running this test will always throw the error if using quasar@2.10.1

I'll make an issue with my reproduction in the quasar repo shortly, I'm trying to find a way to reproduce the race condition in a test repo.

IlCallo commented 1 year ago

If the problem is about Vitest fake timers, open the issue on Vitest first Usually it's the testing tool that should adapt to existing technologies, not viceversa

jdk2pq commented 1 year ago

I know this issue was closed a while back, but I was also having a similar issue with QDialogs and opened an issue with happy-dom here: https://github.com/capricorn86/happy-dom/issues/642. A PR just got submitted to fix that up, so hopefully that'll get merged in soon and fix this problem: https://github.com/capricorn86/happy-dom/pull/797