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.

Sign up for GitHub

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

Jump to bottom

chore: Distribute tests to desktop-gui containers. Make desktop-gui tests faster! #21305

Merged
merged 4 commits into from
May 6, 2022
Merged

chore: Distribute tests to desktop-gui containers. Make desktop-gui tests faster! #21305

merged 4 commits into from
May 6, 2022

Conversation

sainthkh
Copy link
Contributor

@sainthkh sainthkh commented May 3, 2022 •
edited by flotwig
Loading

User facing changelog

N/A. It's an internal CI change.

Additional details

Why was this change necessary?

For some reason, a test in settings_spec.js of desktop-gui CI failed on all recent PRs of mine (#21286, #21285, #21197, #21283). So, I tested it with #21304 that it happens with an empty commit.

I investigated this failure and learned that it's a flaky test and run 7 times (5 passed, 2 failed). Because each container of desktop-gui is running all of the desktop-gui tests.

It was the reason why desktop-gui is always super late to finish.

And it can cause unwanted flakiness.

What is affected by this change?

N/A

Any implementation details to explain?

  • I distributed tests to the containers. Currently, each container runs 3 test suites.
  • I used the code at driver tests.
  • It's much faster to run CI. desktop-gui finishes before others.

How has the user experience changed?

N/A.

PR Tasks

  • [na] Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?
  • [na] Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 3, 2022

Thanks for taking the time to open a PR!

@sainthkh sainthkh changed the title chore: 7x-desktop-gui chore: Distribute tests to desktop-gui containers. May 3, 2022
@sainthkh sainthkh marked this pull request as ready for review May 3, 2022 07:16
@sainthkh sainthkh changed the title chore: Distribute tests to desktop-gui containers. chore: Distribute tests to desktop-gui containers. Make desktop-gui tests faster! May 3, 2022
flotwig
flotwig suggested changes May 3, 2022
View reviewed changes
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

Hey @sainthkh, nice fix! We actually use Cypress parallelization, but that won't work for contributors since there is no MAIN_RECORD_KEY, hence the problem.

Can you update your PR to use the approach described in #21318 please? i.e. use cypress run --record --parallel if MAIN_RECORD_KEY is available, and circleci tests split if not.

@sainthkh
Copy link
Contributor Author

sainthkh commented May 5, 2022 •
edited
Loading

@flotwig I checked the CI and it seems that desktop-gui-integration and runner-integration are using multiple containers.

reporter-integration, ui-components-integration also use MAIN_RECORD_KEY, but they don't use multiple containers yet. So, I didn't change anything for them.

I tried to change runner-integration, too. But for some reason, CI cannot retrieve Cypress binary in runner tests. I reverted it. It isn't that slow and flaky. It isn't that important for now, I think.

@sainthkh
Revert "runner config."
9a3eecc 
This reverts commit b5d439b.
flotwig
flotwig approved these changes May 5, 2022
View reviewed changes
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

nice, thank you for fixing this slowness @sainthkh

@flotwig flotwig requested a review from ZachJW34 May 5, 2022 15:25
ZachJW34
ZachJW34 approved these changes May 6, 2022
View reviewed changes
Copy link
Contributor

@ZachJW34 ZachJW34 left a comment

Choose a reason for hiding this comment

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

CI looks good!

@flotwig flotwig merged commit d1436ee into cypress-io:develop May 6, 2022
@cypress-bot cypress-bot bot mentioned this pull request May 6, 2022
Closed
tgriesser added a commit that referenced this pull request May 13, 2022
@tgriesser
Merge branch '10.0-release' into tgriesser/feat/remoteField-stitching
7383e79 
* 10.0-release: (22 commits)
  fix: migrate multiples projects when in global mode (#21458)
  test: fix flaky cy-in-cy selector validity test (#21360)
  chore: remove unused codeGenGlobs (#21438)
  fix: use correct path for scaffolding spec on CT (#21411)
  fix: remove breaking options from testing type on migration (#21437)
  fix: test-recording instructions in Component Test mode (#21422)
  feat: distinguish app vs launchpad utm_source when using utm params (#21424)
  chore: update stubbed cloud types (#21451)
  chore: change to yarn registry
  fix(sessions): refactor flows, fix grouping bugs and align validation fail text (#21379)
  chore(sessions): more driver tests (#21378)
  chore: rename domain_fn to origin_fn (#21413)
  chore: release 9.6.1 (#21404)
  fix: ensure that proxy logs are updated after the xhr has actually completed (#21373)
  chore: Re-organize tests in assertions_spec.js (#21283)
  chore: Distribute tests to desktop-gui containers. Make `desktop-gui` tests faster! (#21305)
  chore(sessions): add additional tests (#21338)
  fix: Allow submit button to be outside of the form for implicit submission (#21279)
  fix(launcher): support Firefox as a snap (#21328)
  chore(sessions): break out sessions manager code and add unit tests (#21268)
  ...
This was referenced Jun 14, 2022
Merged
Closed
@sainthkh sainthkh mentioned this pull request Jun 15, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZachJW34 ZachJW34 ZachJW34 approved these changes

@flotwig flotwig flotwig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI: Builds from external contributors duplicate tests run across mutliple machines
4 participants
@sainthkh @flotwig @ZachJW34 @mjhenkes