Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallelize Cypress tests #18083

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

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

Description

Try to run Cypress E2E tests in parallel

@cconard96
Copy link
Contributor Author

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.

Copy link
Contributor

@AdrienClairembault AdrienClairembault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding parallelization in general, we need to do more work than just dropping in a library or we may end up with even more flakiness.

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.

A reasonable setup for GLPI would be that each process run with a separate user in a separate entity.
Then we add another separate single threaded execution at the end for tests that modify global state beyond entities (like display preferences or GLPI's config values) and are thus too risky to be run concurrently.

Regarding the library itself, two concerns on my side:

  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).
  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)

@cconard96
Copy link
Contributor Author

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 (#16373 (comment)).

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.

@AdrienClairembault
Copy link
Contributor

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 (#16373 (comment)).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants