glpi icon indicating copy to clipboard operation
glpi copied to clipboard

Parallelize Cypress tests

Open cconard96 opened this issue 1 year ago • 1 comments

Checklist before requesting a review

Please delete options that are not relevant.

  • [x] I have read the CONTRIBUTING document.
  • [x] I have performed a self-review of my code.

Description

Try to run Cypress E2E tests in parallel

cconard96 avatar Oct 17 '24 11:10 cconard96

Without any optimization from a weights file: 20 min -> 14 min (30% faster) Haven't been able to create a weights file because the entity selector test almost always fails locally and the weights file gets generated only after all tests complete successfully.

There are a ton of tests for the Forms feature and they are all fairly slow. Not sure if there are performance improvements that can/need to be made in the code or if there are any improvements that can be done in the tests.

cconard96 avatar Oct 17 '24 12:10 cconard96

Indeed, when running parallelized e2e tests, you need to make sure there wont be any flakiness due to the application state being read and/or modified by multiple process at the same time. For example, the tests of the helpdesk home page validate that only 5 columns are shown in the ticket list and would potentially fail if the display preference tests is ran at the same time as it add extra columns temporarily.

None of the tests should have been written that way. They all were supposed to be permissive because they could be run multiple times locally without resetting the DB. I know there are some tests already that fail from running multiple times. We discussed this in the initial Cypress PR (https://github.com/glpi-project/glpi/pull/16373#issuecomment-1997167779).

1. The package is still in beta (0.14) and does not seems to be maintained anymore (no commits since 2023 while having a lot of opened issues).

Tabler is over 6 years old, still in beta, and hasn't had a release in two years. Not gonna judge a library solely on that. If it is stable and does what we need it to, its not really a big deal for a dev tool to go a year or so without an update. It isn't necessarily a long-term solution anyways. Improvements to GLPI itself should be made.

2. Cypress has been killing alternative to its cloud when they get too popular and can drop support for this extension very easily (see [Breaking change without warning cypress-io/cypress#28269](https://github.com/cypress-io/cypress/issues/28269))

Two specific cloud solutions from the same author that used the cypress-cloud code and slapped a different name on it. What Cypress did in response doesn't seem entirely unreasonable and I'm not commenting on legal standing. This library is just a local wrapper for Cypress essentially. It groups the specs together and then launches multiple Cypress processes which handle a specific group of specs and then compiles the results together.

cconard96 avatar Oct 28 '24 17:10 cconard96

None of the tests should have been written that way. They all were supposed to be permissive because they could be run multiple times locally without resetting the DB. I know there are some tests already that fail from running multiple times. We discussed this in the initial Cypress PR (https://github.com/glpi-project/glpi/pull/16373#issuecomment-1997167779).

This is not the same subject. The tests being able to be executed multiples times without resetting the database doesn't mean that they can run all at once using multiple threads.

Lets say you modify the "Go to created item after creation" setting. Your test can be executed multiple times and won't impact other tests in a single thread context if you reset the setting at the end of the test. In a multi-threaded context, you may get another test that will run at the same time and will be impacted by this setting before you have time to reset it.

This is an inherent issue of modifying server state during your tests.

Tabler is over 6 years old, still in beta, and hasn't had a release in two years. Not gonna judge a library solely on that. If it is stable and does what we need it to, its not really a big deal for a dev tool to go a year or so without an update. It isn't necessarily a long-term solution anyways. Improvements to GLPI itself should be made.

Not having a release is not the same as not having any commit done. There are open issues where people ask for write access to the repository to be given to someone else because the owner has not interacted with it for almost a year and their contributions can't be merged.

AdrienClairembault avatar Oct 29 '24 09:10 AdrienClairembault

Just to add a bit to the conversation: what about https://github.com/sorry-cypress/sorry-cypress ? We may order a vps if it's a solution to the current issue

orthagh avatar Nov 04 '24 19:11 orthagh

Just to add a bit to the conversation: what about https://github.com/sorry-cypress/sorry-cypress ? We may order a vps if it's a solution to the current issue

Sorry Cypress and Currents (The paid cloud service for Sorry Cypress) are the two solutions that Cypress.io claimed trademark infringement and added some checks to their 'cypress-cloud' library to prevent it from working with these solutions. From what I understand, you run cypress-cloud with this solution and point it to the library to handle certain parts. The solutions allegedly profited from using the Cypress/Cypress Cloud/Dashboard branding. AFAIK Sorry-Cypress should support Cypress 13 now, but Currents is heavily marketing Playwright compatibility and pushing Cypress support out of sight and it says they suspended work on Cypress 13 support.

I don't think there is any noticeable difference in performance between the solution in this PR and Sorry-Cypress. The limiting factors are GLPI and the test code IMO; All of which can be addressed.

We should make sure it is testing mostly just the UI and not unnecessarily covering PHP/JS unit test stuff. If features like Forms were written in Vue, we could have set it up as a component test which would load a bare minimum page with just the editor component mounted which would be quicker from ignoring all the unneeded GLPI stuff.

cconard96 avatar Nov 05 '24 11:11 cconard96

As discussed with @orthagh, we will soon be trying to replace cypress with playwright. Therefore, we should not spend any more time trying to improve the execution time of the cypress test suite.

cedric-anne avatar Dec 10 '24 10:12 cedric-anne