Skip to content

Upgrade Cypress from 9 to 13 for FTRepo - #1988

Closed
yyfamazon wants to merge 12 commits into
opensearch-project:mainfrom
yyfamazon:main
Closed

Upgrade Cypress from 9 to 13 for FTRepo#1988
yyfamazon wants to merge 12 commits into
opensearch-project:mainfrom
yyfamazon:main

Conversation

@yyfamazon

@yyfamazon yyfamazon commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Description

Upgrade Cypress from 9 to 13 for FTRepo

baseline: #1895
I compare the ci test with baseline, and fix all tests that passed in baseline in this pr.

Issues Resolved

#1996

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: yyfamazon <yyf@amazon.com>
@yyfamazon
yyfamazon force-pushed the main branch 5 times, most recently from 21e47ed to 15d7bca Compare April 14, 2026 09:44
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Comment thread cypress/plugins/index.js
path: parsed.pathname + parsed.search,
method,
headers,
rejectUnauthorized: false,
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Signed-off-by: yyfamazon <yyf@amazon.com>
Comment thread package.json Outdated
"@cypress/skip-test": "^2.6.1",
"@opensearch-dashboards-test/opensearch-dashboards-test-library": "https://github.com/opensearch-project/opensearch-dashboards-test-library/archive/refs/tags/1.0.6.tar.gz",
"brace": "^0.11.1",
"cypress": "^13.6.3",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"cypress": "^13.6.3",
"cypress": "13.6.3",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have created a new branch in my fork based on your main for testing purpose.
https://github.com/peterzhuamazon/opensearch-dashboards-functional-test/tree/cypress13-test

If there is more changes I will update there and PR to your fork. Thanks.

PSA: The above version should be locked, this is for easier setups and version matching specifically for docker images on Jenkins. Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks

Signed-off-by: yyfamazon <yyf@amazon.com>
@peterzhuamazon

peterzhuamazon commented Apr 22, 2026

Copy link
Copy Markdown
Member

Hi All,


       Spec                                              Tests  Passing  Failing  Pending  Skipped
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  0_add_saved_object.js                    00:50        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  1_import_vector_map_tab.spec.js          00:50        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  2_opensearchMapLayer.spec.js             01:35        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  3_add_saved_object.spec.js               01:01        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  4_documentsLayer.spec.js                 02:32        2        2        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  5_add_map_to_dashboard.spec.js           01:13        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  6_geojson_file_upload.spec.js            00:52        1        1        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  7_enable_new_home_ui.spec.js             01:04        2        2        -        -        - │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        10:03       10       10        -        -        -

Run a few test locally with 13.17.0 image and seems ok.
We can only find issues once it is up to Jenkins.

Once https://github.com/yyfamazon/opensearch-dashboards-functional-test/pull/1 is merged I think we can merge this PR then I will run full stack on all dists on jenkins.

If there is more issues I can contact @yyfamazon to fix.

Thanks!

@peterzhuamazon

peterzhuamazon commented Apr 22, 2026

Copy link
Copy Markdown
Member

Signed-off-by: yyfamazon <yyf@amazon.com>
@peterzhuamazon

peterzhuamazon commented Apr 28, 2026

Copy link
Copy Markdown
Member

Still pending this PR to be merged related to above comment.

@wanglam

wanglam commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

Review: Upgrade Cypress from 9 to 13

Thanks for the work on this migration, @yyfamazon! The overall approach is solid. Here are my findings:

Must Fix

1. Remove cypress/downloads/downloads.html (2.62 MB binary)
This looks like a Cypress test artifact that was accidentally committed. It should be removed from the PR and added to .gitignore under cypress/downloads/.

Should Fix

2. Document CYPRESS_SKIP_HEAVY_DISCOVER_TESTS
This new environment variable was added to assistant-release-e2e-workflow.yml but there's no documentation on what tests it skips or why it's needed. Please add context — either in the PR description or as a code comment.

Informational / Minor

3. rejectUnauthorized: false in cypress/plugins/index.js
GitHub Advanced Security flagged this. I assume this is intentional for connecting to local OpenSearch instances with self-signed certs during testing, but worth confirming it was already present before this PR and not a regression.

4. testIsolation: false
This preserves the Cypress 9 behavior where tests within a spec share state. Makes sense for this migration to avoid breaking everything, but worth tracking as tech debt — order-dependent tests are fragile.

5. "cypress/unsafe-to-chain-command": "off" in .eslintrc.json
Pragmatic for a migration of this scale. Consider enabling it in a follow-up PR and fixing violations incrementally.

6. Cypress version pinning
I see @peterzhuamazon already flagged changing "^13.6.3" to "13.6.3" — just confirming this was applied in the latest commit.

What looks good

  • Clean migration from cypress.json to cypress.config.js using defineConfig()
  • Memory optimization flags for Chromium in CI (--max-old-space-size=4096, experimentalMemoryManagement, numTestsKeptInMemory: 0) — should help with CI stability
  • Plugin wiring through setupNodeEvents with a try/catch fallback
  • Documentation updates in DEVELOPER_GUIDE.md and ONBOARDING.md to reference the new config file
  • The sed 's/,$//' fix in the CI groups workflow for trailing comma handling

Overall this is in good shape. The binary file removal is the only blocker from my side. 👍

@peterzhuamazon

peterzhuamazon commented Apr 30, 2026

Copy link
Copy Markdown
Member

Hi @wanglam @yyfamazon,

I have mentioned in PM and in my https://github.com/yyfamazon/opensearch-dashboards-functional-test/pull/1 PR to @yyfamazon branch that we need 13.17.0

Could this please be added, you can see in my last two comments, I have mentioned this change needs to be onboarded.

(additionally I said 13.17.0 not 13.6.3 is because ^13.6.3 is already resolved to 13.17.0, in my local test and in github check so I just bump it and fix to 13.17.0)

Let me know,

Thanks.

After sync with @yyfamazon this change is already in with c18b6d9.

Then the PR is just pending for review and merge.

Thanks.

@peterzhuamazon

Copy link
Copy Markdown
Member

Hi @yyfamazon @ruanyl @wanglam are we ready to merge this and start testing?

Thanks!

@yyfamazon

Copy link
Copy Markdown
Contributor Author

Hi @wanglam @yyfamazon,

I have mentioned in PM and in my yyfamazon#1 PR to @yyfamazon branch that we need 13.17.0

Could this please be added, you can see in my last two comments, I have mentioned this change needs to be onboarded.

(additionally I said 13.17.0 not 13.6.3 is because ^13.6.3 is already resolved to 13.17.0, in my local test and in github check so I just bump it and fix to 13.17.0)

Let me know,

Thanks.

After sync with @yyfamazon this change is already in with c18b6d9.

Then the PR is just pending for review and merge.

Thanks.

@yyfamazon

Copy link
Copy Markdown
Contributor Author

Hi @yyfamazon @ruanyl @wanglam are we ready to merge this and start testing?

Thanks!
@peterzhuamazon yes, please merge and start testing, thanks, I dont have merge access.

@peterzhuamazon

Copy link
Copy Markdown
Member

@yyfamazon did you remove your branch and cause it to close?
Or are you planning another PR?
Thanks.

@peterzhuamazon

Copy link
Copy Markdown
Member

Talked to @yyfamazon seems like no one knows why it is closed, probably due to github outage a few days ago.
Asking him to open a new PR.

Thanks.

@yyfamazon

Copy link
Copy Markdown
Contributor Author

Remove cypress/downloads/downloads.html
removed Remove cypress/downloads/downloads.html in the newest PR. #2000

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.

5 participants