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

Upgrades cypress to 13.15.1 #198715

Merged
merged 11 commits into from
Nov 26, 2024
Merged

Conversation

michaelolo24
Copy link
Contributor

@michaelolo24 michaelolo24 commented Nov 1, 2024

Summary

This PR upgrades cypress to 13.15.1. This is needed to fix an very annoying issue where Cypress tests hangs when ran locally and when the window is not focused

See Cypress issue here: cypress-io/cypress#28392

@michaelolo24 michaelolo24 added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. ci:all-cypress-suites v8.17.0 labels Nov 1, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@michaelolo24 michaelolo24 force-pushed the upgrade-cypress branch 2 times, most recently from ae5760c to 574336a Compare November 5, 2024 21:11
@michaelolo24 michaelolo24 requested a review from a team as a code owner November 5, 2024 21:11
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Nov 5, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

Copy link
Contributor

github-actions bot commented Nov 5, 2024

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@michaelolo24 michaelolo24 force-pushed the upgrade-cypress branch 3 times, most recently from 2177bcc to 4c23685 Compare November 7, 2024 00:11
@cauemarcondes
Copy link
Contributor

@michaelolo24 The APM cypress tests failing is a known issue and we'll fix that soon. Sorry for that.

@michaelolo24
Copy link
Contributor Author

@michaelolo24 The APM cypress tests failing is a known issue and we'll fix that soon. Sorry for that.

Thanks @cauemarcondes ! Since I would rather not skip tests, I'll wait for those issues to be fixed and then will push this upgrade. Thank you!

@michaelolo24
Copy link
Contributor Author

@cauemarcondes - We actually may end up skipping them to get the upgrade in as running cypress locally has become increasingly difficult. I tried to be judicious in skipping only the specific tests that we're failing rather than whole blocks.

If it helps, I think the fix for service_overview, service_inventory, and agent_configuration files may involve either updating the input/combobox components for environmentFilter and serviceNameComboBox to be more easily modifiable or updating the tests to not rely on typing in them

For the integration_polciy and tutorial files, the pages don't seem to be loading the views properly so not sure what's happening there

@cauemarcondes
Copy link
Contributor

I've got a PR skipping the problematic tests: #199336

@cauemarcondes
Copy link
Contributor

@michaelolo24 my PR has been merged.

@PhilippeOberti
Copy link
Contributor

@michaelolo24 my PR has been merged.

thanks! I rebased this PR accordingly

@PhilippeOberti
Copy link
Contributor

@cauemarcondes can you take one more look and review this PR? The build is green it's ready to go and we need an approval from the @elastic/obs-ux-infra_services-team.
Thanks!

@@ -88,7 +88,7 @@ describe('Service inventory', () => {
cy.visitKibana(serviceInventoryHref);
});

it('with the correct environment when changing the environment', () => {
it.skip('with the correct environment when changing the environment', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@michaelolo24 these tests are not failing on main AFAICT

Copy link
Contributor

Choose a reason for hiding this comment

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

I unskipped all the tests, let's see what the build says!

Copy link
Contributor

@PhilippeOberti PhilippeOberti Nov 14, 2024

Choose a reason for hiding this comment

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

@cauemarcondes over the last 2 builds (this one and that one) we have 3 tests failing:

  • service_inventory/service_inventory.cy.ts
  • service_overview/service_overview.cy.ts
  • settings/agent_configurations.cy.ts

These are the 3 tests that @michaelolo24 had skipped and I unskipped in my last commit.
If these are not failing on main, can you help us figuring out why these 3 tests would consistently fail?

Copy link
Contributor

@cauemarcondes cauemarcondes Nov 18, 2024

Choose a reason for hiding this comment

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

@PhilippeOberti / @michaelolo24 after checking out your branch and running the test I could not see any specific problem with the tests you mentioned. They are breaking, as CI pointed, but there is no clear reason why. This is the error showed on the console:

2024-11-18 11:58:49.047 Cypress Helper (Renderer)[30120:5226514] CoreText note: Set a breakpoint on CTFontLogSystemFontNameRequest to debug.

We detected that the Electron Renderer process just crashed.

We have failed the current spec but will continue running the next spec.

This can happen for a number of different reasons.

If you're running lots of tests on a memory intense application.
  - Try increasing the CPU/memory on the machine you're running on.
  - Try enabling experimentalMemoryManagement in your config file.
  - Try lowering numTestsKeptInMemory in your config file during 'cypress open'.

You can learn more here:

https://on.cypress.io/renderer-process-crashed

  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        1                                                                                │
  │ Passing:      0                                                                                │
  │ Failing:      1                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  0                                                                                │
  │ Video:        true                                                                             │
  │ Duration:     11 seconds                                                                       │
  │ Spec Ran:     service_inventory.cy.ts                                                          │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘

I double-cheked and the tests are indeed passing on the main branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I remember @michaelolo24 added this experimentalMemoryManagement to the config file (here) but I think Glo said it wasn't in the right place.
I just pushed a commit moving it to the e2e location, hopefully that will do the trick here... if not I don't know what to do...

Copy link
Contributor

Choose a reason for hiding this comment

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

considering there are only 3 files modified in this PR, 2 of them are package.json and its lock file, and the third one being the experimentalMemoryManagement change to the config file to try to fix the failing tests, I think it's safe to say that the failures are due to the Cypress update...
If we want to continue updating Cypress, I think we need to re-work these 3 tests a bit to make them work with the newer version.

Copy link
Contributor

Choose a reason for hiding this comment

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

This version works. Although we're not doing like the previous test, we are just opening the combo box and selecting the element.

it('with the correct environment when changing the environment', () => {
      cy.intercept('GET', '/internal/apm/suggestions?*').as('environmentSuggestionOptions');
      cy.wait(mainAliasNames);
      cy.wait('@environmentSuggestionOptions');

      cy.getByTestSubj('environmentFilter').click();
      cy.contains('button', 'production').click();

      cy.expectAPIsToHaveBeenCalledWith({
        apisIntercepted: mainAliasNames,
        value: 'environment=production',
      });
    });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @cauemarcondes, makes sense ! Would you be willing to commit the changes for the three files directly to the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @michaelolo24 I was trying to fix the tests but the browser keeps crasing all the time with this message:

We detected that the Chrome Renderer process just crashed.

We have failed the current spec but will continue running the next spec.

This can happen for a number of different reasons.

If you're running lots of tests on a memory intense application.
  - Try increasing the CPU/memory on the machine you're running on.
  - Try enabling experimentalMemoryManagement in your config file.
  - Try lowering numTestsKeptInMemory in your config file during 'cypress open'.

You can learn more here:

https://on.cypress.io/renderer-process-crashed
Screenshot 2024-11-19 at 14 27 51

Unfortunatly I don't feel comfortable approving this PR until this is fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, this goes back to the error I was seeing when interacting with the environmentFilter dropdown. Suspecting cypress crashes due to an infinite loop of trying to delete the field values, but not being able to delete...but not 100% sure. I was able to get the changes I made working locally, but because of the way the component works, it's hard to properly test the suggestion api since you can't clear the environment field. We'll see if this current ci run passes. If so, please feel free to make modifications you'd like to make to my changes to better fit APM's expectations as the tests shouldn't crash any longer 🤞🏾

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#7478

[❌] APM - Cypress: 20/25 tests passed.

see run history

@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 26, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: 117565d
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-198715-117565d9c729

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #19 / Category can submit without setting a category

Metrics [docs]

✅ unchanged

History

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7480

[✅] APM - Cypress: 25/25 tests passed.

see run history

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelolo24 michaelolo24 merged commit 1865869 into elastic:main Nov 26, 2024
63 checks passed
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
## Summary

This PR upgrades cypress to `13.15.1`. This is needed to fix an very
annoying issue where Cypress tests hangs when ran locally and when the
window is not focused

See Cypress issue here:
cypress-io/cypress#28392

---------

Co-authored-by: PhilippeOberti <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
## Summary

This PR upgrades cypress to `13.15.1`. This is needed to fix an very
annoying issue where Cypress tests hangs when ran locally and when the
window is not focused

See Cypress issue here:
cypress-io/cypress#28392

---------

Co-authored-by: PhilippeOberti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:all-cypress-suites ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants