quasarframework / quasar-testing

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

Simple select component randomly fails test #343

Closed lavoscore closed 4 months ago

lavoscore commented 1 year ago

I have a very simple QSelect based component that will occasionally fail as image below:

image

It happens very frequently during CI (cypress run), less so via cypress open, and I can't find the cause. The last green message makes me think that Cypress is able to find the popup menu, but even though I can always see its options in the test, withinSelectMenu can't find them inside the menu, failing here:

https://github.com/quasarframework/quasar-testing/blob/d63b63096a8ec4c3a8847f5407ef196469e8ab04/packages/e2e-cypress/src/helpers/commands/cypress-overrides.ts#L43

Why would that happen only occasionally? Since withinSelectMenu "assumes there's a single select menu open at any time", I wonder if there could be some extra empty popup left open somewhere, maybe not correctly unmounted by Cypress between tests or something. I'd appreciate advise on how to narrow this bug down further.

Test:

const tipo = ref('PR');

it('v-model works', () => {
  cy.mount(SeletorDeTipo, { props: { ...vModelAdapter(tipo) } });

  cy.dataCy('campo-tipo').closest('.q-select').select('PLL');
  cy.then(() => expect(tipo.value).to.eq('PLL'));
});

Component:

<template>
  <q-select
    label="Tipo"
    v-model="tipo"
    :options="tipos"
    clearable
    data-cy="campo-tipo"
  />
</template>

<script setup lang="ts">
import { computed } from 'vue';

const props = defineProps<{
  modelValue: string | undefined;
}>();

const emit = defineEmits<{
  (e: 'update:modelValue', valor: string | undefined): void;
}>();

const tipos = ['PLL', 'PDL', 'PR', 'IND']

const tipo = computed({
  get: () => props.modelValue,
  set: (valor) => emit('update:modelValue', valor),
});
</script>

I'm using version 5.1 and Cypress 12.11.0.

david-mohr commented 10 months ago

I'm seeing the same issue with the example test for QuasarSelect. As mentioned above, it happens more frequently during cypress run, and rarely if it's on its own (using cypress run -s).

I've found a potential workaround which is to click on the select twice after mounting (once to open, once to close) and the error hasn't happened since.

IlCallo commented 10 months ago

I wonder if there could be some extra empty popup left open somewhere

This is probably the case, or something is somewhat forcing your select to close when it shouldn't In some cases it could be due to the animation of another Select or component not having finished their outro animation, but this doesn't seem to be the case

Could you upgrade Vue, Quasar and Cypress deps and let me know if it still occurs? From what I can see in the image, the q-menu isn't actually visible, which can very likely be the cause of the issue Maybe a change in the select options or modelValue is closing it somehow?

@david-mohr are you able to replicate this on a clean new project? I'm apparently not able to

IlCallo commented 10 months ago

We just noticed the same problem in our own Quasar UI test suite, we're investigating the problem

david-mohr commented 10 months ago

@IlCallo yes, I can replicate with a new project. It's inconsistent, but one way I've found to trigger it is to run the test just on QuasarSelect, hit CTRL+C as soon as the second test passes, then run it again

$ yarn test:component:ci -s src/components/__tests__/QuasarSelect.cy.js
...
  QuasarSelect
    ✓ makes sure the select is disabled (116ms)
    ✓ selects an option by content (533ms)
# hit CTRL+C here

$ yarn test:component:ci -s src/components/__tests__/QuasarSelect.cy.js
...
  QuasarSelect
    ✓ makes sure the select is disabled (169ms)
    1) selects an option by content
n05la3 commented 10 months ago

I checked this out with @IlCallo. In my case, I didn't have any issues initially when running on node 16. I then switched to both 18 and 20 and started experiencing similar random errors. Like 3 fails on every 5 runs. Hours later, I could not get any errors again on either 16, 18 or 20 after several trials. @lavoscore, @david-mohr, could you confirm if switching the node version makes any difference?

david-mohr commented 9 months ago

@n05la3 I found that switching the node version did NOT make any difference (I got failures in all three). FYI I simply used nvm to switch between versions, e.g. nvm use v16

lavoscore commented 9 months ago

Maybe it's too soon to celebrate yet, but after upgrading stuff I'm currently unable to reproduce the original behavior. I still see an occasional flake (specific to another test), but had to cypress run a lot to see it. Here's my current dependencies:

"dependencies": {
    "@quasar/extras": "^1.16.3",
    "axios": "^1.5.1",
    "core-js": "^3.30.2",
    "quasar": "^2.12.7",
    "ts-loader": "^9.4.2",
    "vue": "^3.3.4",
    "vue-router": "^4.2.5"
  },
  "devDependencies": {
    "@faker-js/faker": "^8.1.0",
    "@quasar/app-webpack": "^3.11.2",
    "@quasar/quasar-app-extension-testing-e2e-cypress": "^5.1.1",
    "@testing-library/cypress": "^10.0.1",
    "@types/humps": "^2.0.2",
    "@types/node": "^20.1.1",
    "@typescript-eslint/eslint-plugin": "^5.10.0",
    "@typescript-eslint/parser": "^5.10.0",
    "cypress": "^13.3.0",
    "fishery": "^2.2.2",
    "mock-socket": "^9.3.1",
    "prettier": "^2.5.1",
    "typescript": "^5.2.2",
    "workbox-webpack-plugin": "^7.0.0"
  },

Using Node 18.16.0.

Anybody else seeing progress with a similar setup?

IlCallo commented 9 months ago

Which deps have you bumped during your upgrade? And from which previous version?

sodatea commented 9 months ago

I've looked into this issue a while back but with no luck.

The best I can guess is that it's related to QMenu animations. Because in QMenu.cy.js, all click() calls have waitForAnimations: true enabled, while in QSelect.cy.js, though the component uses QMenu underlyingly, none is called with waitForAnimations: true.

Reference: https://docs.cypress.io/guides/core-concepts/interacting-with-elements#Animations

But this issue is so hard to debug: it never fails on my MacBook Pro with M1 Pro, so I can only test it in GitHub Actions. https://github.com/sodatea/quasar/commits/test-ci-qselect I've tried modifying the global Cypress configurations, but the tests still failed randomly. Without the modified configuration, in Vue Ecosystem CI, the failing test case was always the (prop): emit-value one. After the modification, it became some other random test cases in the same file.

So I can only assume it's related to animations, but never sure how to actually fix it.

IlCallo commented 9 months ago

Thanks for reporting your finding, we'll try playing around with waitForAnimations and see if we can solve this at its root cause

lavoscore commented 9 months ago

Which deps have you bumped during your upgrade? And from which previous version? - @IlCallo

dependencies:

"axios": "^1.4.0",
"quasar": "^2.12.0",
"vue": "^3.2.47",
"vue-router": "^4.1.6"

devDependencies:

"@faker-js/faker": "^7.6.0",
"cypress": "^12.11.0",
"@quasar/app-webpack": "^3.9.0",
"@quasar/quasar-app-extension-testing-e2e-cypress": "^5.1.0",
"@testing-library/cypress": "^9.0.0",
"mock-socket": "^9.2.1",
"typescript": "^5.0.4",
"workbox-webpack-plugin": "^6.5.4"
IlCallo commented 9 months ago

I tried to bump deps, but to no avail I'm able to reproduce the problem with upgraded deps in Quasar tests, but not on Cypress AE tests anymore

Debugging this a bit further, I noticed that Quasar tests throw this error

<div#f_0893ba7e-7776-410a-b65d-d830bc1b73d2_lb.q-menu.q-position-engine.scroll.q-menu--square> is not visible because it has CSS property: visibility: collapse

And this issue on Cypress, which doesn't seem that much related: https://github.com/cypress-io/cypress/issues/21164

sodatea commented 9 months ago

In this case, it looks like https://github.com/quasarframework/quasar/blob/3ef2a0a036ad7157b1d69c3d0edb4563c625e433/ui/src/utils/private/position-engine.js#L175 isn't being called when the assertion takes place.

IlCallo commented 9 months ago

I was able to get errors consistently by running

yarn test:component:run --spec "src/components/select/__tests__/QSelect.cy.js"

in Quasar repo ui folder

All errors are due to clicks on elements with visibility: collapse Using waitForAnimations didn't help in any way, but the problem is solved by using force: true on clicks involving .q-menu I guess this is indeed a problem with QMenu animations, but it doesn't really make sense because Cypress should retry and eventually find the correct element, or at least it used to

The problem is that using force: true isn't possible when doing cy.get('.q-menu').should('be.visible') which considers visibility: collapse as not visible either, and it seems it's not retrying the check either

IlCallo commented 9 months ago

Seems like I pin-pointed the problem to this change: https://github.com/quasarframework/quasar/commit/c9e43faf2f12d23935679522ddce5c50271378c2

Not really sure what's going on there, I'll ask for Razvan input on this

IlCallo commented 9 months ago

@sodatea you're right, it seems like the problem is that it gets stuck into the retry loop here:

https://github.com/quasarframework/quasar/blob/3ef2a0a036ad7157b1d69c3d0edb4563c625e433/ui/src/utils/private/position-engine.js#L120-L125

After 5 retries it just returns without doing anything and this leaves the visibility: collapse in place By commenting out that check, tests will pass without problems

Not really sure why this happens tho Apparently, when run in CI mode, cfg.targetEl.offsetHeight or cfg.targetEl.offsetWidth are set to 0 and this breaks the check. What I find strange is that it doesn't happen consistently, that's on Cypress I guess?

n05la3 commented 9 months ago

I can confirm the errors on Quasar tests, I'm also consistently getting similar errors. In a few cases, some of the failing test cases pass.

antonisntoulis commented 7 months ago

I can confirm the issue too, using force: click solves the issue but it's not ideal.

n05la3 commented 4 months ago

I tried digging into this again. It is difficult to pinpoint the culprit since the issue occurs randomly in CI and rarely locally. But it does happen every now and then. I looked into the code but could not locate anything causing this.

I guess it is related to Cypress finding the element, trying to click on it while it is still invisible. Cypress is supposed to check that the element is visible before clicking but it seems not to be doing so for some reason. Adding explicit visibility checks resolves the issues. I have added these explicit visibility checks to the Quasar QSelect component tests and it seems to no longer be failing.

The issue and Cypress blog post below talk about this more and the recommendation in such cases is to explicitly check that the element is visible before clicking.

IlCallo commented 4 months ago

Could you add it automatically to the ".select" override?

And convert both Quasar tests to use that helper if possible

IIRC they have been written before the helper was available

n05la3 commented 4 months ago

Yeah, sure. I'll check that out.

IlCallo commented 4 months ago

Seems like this is still happening in some circumstances

In particular, I'm experiencing it with Cypress 13.7.0 and the bundled Electron 27.1.3 It may just be an out of date Electron version causing this

Tests ran against Firefox or Chromium don't generate that issue, and @n05la3 doesn't experience that problem on his machine, so this could be an env-related problem

We'll proceed as defined here and here and then close this issue for good

@sodatea we fixed QSelect and use-validate tests into the main repo already, if you have some time to test them out and remove this workaround

We also recently migrated Quasar monorepo to pnpm, so this should be good to go too: https://github.com/vitejs/vite-ecosystem-ci/pull/208