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

chore: split cypress files for less memory #30354

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Sep 21, 2024

SUMMARY

Testing to see if this will fix the cypress memory issues.
Also added the chrome flag disable-dev-shm-usage

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho eschutho force-pushed the elizabeth/split-cypress-files branch from 50c30a1 to 650b5c8 Compare September 23, 2024 21:52
@mistercrunch
Copy link
Member

mistercrunch commented Sep 23, 2024

Matrix (1, chrome) failed so I restarted it manually. For the record dashboard/nativeFilters.test.ts is the one that failed, even after shrinking it down as part of this PR. I'm wondering if lowering the limit / cardinality of filters could help here too if the issue comes from too many filters with too high of a cardinality (assuming that this specific test is the hot spot).

Wondering if a wider matrix would help. Also it seems we have 5 * chrome now, but could try running also against Firefox to see if the memory consuption is more or less of a problem on that browser.

@rusackas
Copy link
Member

Matrix (1, chrome) failed so I restarted it manually. For the record dashboard/nativeFilters.test.ts is the one that failed, even after shrinking it down as part of this PR. I'm wondering if lowering the limit / cardinality of filters could help here too if the issue comes from too many filters with too high of a cardinality (assuming that this specific test is the hot spot).

Wondering if a wider matrix would help. Also it seems we have 5 * chrome now, but could try running also against Firefox to see if the memory consuption is more or less of a problem on that browser.

Yes, that is the hot spot, and it's always the one that fails. We've also seen GitHub issues about performance regressions when there are lots of filters, so this failure is likely reflecting a real problem. We can throw memory, split tests, reduce test scope, etc. to smooth out devEx, but I think we're just dancing around a real problem.

@eschutho
Copy link
Member Author

eschutho commented Sep 25, 2024

@rusackas this is passing CI.. I ran it again and had no errors.. can we try to merge it and test it out on master?

@eschutho eschutho merged commit 43721f1 into master Sep 25, 2024
33 checks passed
@rusackas rusackas deleted the elizabeth/split-cypress-files branch September 25, 2024 20:51
@mistercrunch
Copy link
Member

Looked at master hoping to see some green but no :/
Screenshot 2024-09-25 at 2 35 23 PM

@mistercrunch
Copy link
Member

Giving a shot at troubleshooting memory here -> #30397 , seems promising

@sadpandajoe sadpandajoe added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Oct 30, 2024
sadpandajoe pushed a commit that referenced this pull request Oct 30, 2024
@github-actions github-actions bot added 🍒 4.1.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XL v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch 🍒 4.1.0 🍒 4.1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants