Update Cypress configuration and dependencies for improved testing#1251
Update Cypress configuration and dependencies for improved testing#1251ashiduDissanayake wants to merge 9 commits intowso2:mainfrom
Conversation
- Changed test command in package.json to use `--e2e` for Cypress runs. - Updated test specifications to reflect new file structure in package.json. - Upgraded Cypress version from 9.7.0 to 14.3.2 for enhanced features and stability. - Replaced `aws-sdk` with `@aws-sdk/client-s3` and added `archiver` and `nodemailer` dependencies. - Modified local test script to align with new file naming conventions. - Updated Node.js setup script in test.sh to use Node.js 22.x.
ashiduDissanayake
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated CI and test infrastructure: JDK → 21, Node → 22, Cypress upgraded to v14 with new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/test.sh (2)
144-144:⚠️ Potential issue | 🟠 Major
aws-sdkv2 is still installed here, butpackage.jsonmigrated to@aws-sdk/client-s3v3.Line 144 runs
npm i --save aws-sdk, which installs the deprecated AWS SDK v2. This contradicts thepackage.jsonchange that replacedaws-sdkwith@aws-sdk/client-s3. Update this line to match.Proposed fix
-npm i --save aws-sdk +npm i --save `@aws-sdk/client-s3`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test.sh` at line 144, Replace the npm install command that installs the deprecated v2 SDK: locate the line that runs "npm i --save aws-sdk" in tests/test.sh and change it to install the v3 modular package used in package.json (for example install "@aws-sdk/client-s3" instead of "aws-sdk"); ensure the test script now uses the v3 client package name so the installed dependency matches package.json.
103-118:⚠️ Potential issue | 🔴 CriticalNode.js 22.x setup is immediately overridden by a Node.js 12 binary download.
Line 103 runs the NodeSource
setup_22.xscript, but lines 114–117 then download and symlink Node.js v12.22.3 binaries, overriding whatever was set up. The actual runtime will be Node 12, not Node 22, defeating the purpose of this change.Update lines 114–117 to download and symlink a Node.js 22.x tarball (or remove the manual download entirely if the NodeSource setup already installs Node 22 via
apt-get install nodejs).Proposed fix (use Node 22)
-wget https://nodejs.org/dist/v12.22.3/node-v12.22.3-linux-x64.tar.xz -tar -xvf node-v12.22.3-linux-x64.tar.xz -sudo ln -s $HOME/node-v12.22.3-linux-x64/bin/node /usr/bin/node -sudo ln -s $HOME/node-v12.22.3-linux-x64/bin/npm /usr/bin/npm -sudo ln -s $HOME/node-v12.22.3-linux-x64/bin/npx /usr/bin/npx +wget https://nodejs.org/dist/v22.14.0/node-v22.14.0-linux-x64.tar.xz +tar -xvf node-v22.14.0-linux-x64.tar.xz +sudo ln -sf $HOME/node-v22.14.0-linux-x64/bin/node /usr/bin/node +sudo ln -sf $HOME/node-v22.14.0-linux-x64/bin/npm /usr/bin/npm +sudo ln -sf $HOME/node-v22.14.0-linux-x64/bin/npx /usr/bin/npx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test.sh` around lines 103 - 118, The script runs the NodeSource setup_22.x installer with curl but then downloads and symlinks a Node v12 tarball (node-v12.22.3-linux-x64.tar.xz), which overrides the intended Node 22 runtime; either replace the manual download/symlink of node-v12... with a Node 22 tarball (e.g., node-v22.x-linux-x64.tar.xz) and update the symlink targets, or remove the wget/tar/ln -s block entirely and instead install the distro package produced by the setup_22.x step via apt-get install nodejs so the system ends up running Node 22 as intended. Ensure you update the referenced filenames (node-v12.22.3-linux-x64 -> node-v22.*-linux-x64) and the symlink targets (/usr/bin/node /usr/bin/npm /usr/bin/npx) accordingly if you choose the tarball approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ui-test.yml:
- Line 78: Update the GitHub Actions step that references the deprecated action
version by replacing the uses: cypress-io/github-action@v4 entry with a modern,
compatible tag such as uses: cypress-io/github-action@v7 (or `@v6`) so the
workflow runs on a Node.js 18+ runtime and can install Cypress 14+; ensure any
related step inputs/arguments remain valid for the newer action and run the
workflow to confirm no other breaking changes are needed.
In `@tests/cypress.config.js`:
- Line 7: The config contains an unrecognized key largeTimeout in
tests/cypress.config.js which Cypress will ignore; remove the largeTimeout line
(or replace it with the appropriate recognized timeout such as
defaultCommandTimeout, pageLoadTimeout, requestTimeout, responseTimeout,
execTimeout, or taskTimeout) so the timeout setting is applied correctly.
In `@tests/package.json`:
- Line 28: Remove the deprecated dependency "@cypress/browserify-preprocessor"
from tests/package.json and replace its usage in tests/cypress/plugins/index.js
by switching to "@cypress/webpack-preprocessor" (or the Cypress default webpack
bundler) — update package.json dependencies to add
"@cypress/webpack-preprocessor" if chosen, modify the plugin setup in
tests/cypress/plugins/index.js to instantiate and use the webpack preprocessor
instead of the browserify preprocessor (or remove the custom file:preprocessor
hook to use the default bundler), and run the test bootstrap to verify the new
preprocessor integration.
In `@tests/test.local.sh`:
- Line 1: Add a shebang as the first line of tests/test.local.sh so the script
has a defined shell interpreter (e.g., use "#!/usr/bin/env bash" or
"#!/bin/bash") before the existing command "./node_modules/.bin/cypress run
--headed --spec cypress/e2e/admin/08-add-api-categories.cy.js"; ensure the
shebang is the very first line and the file is executable.
---
Outside diff comments:
In `@tests/test.sh`:
- Line 144: Replace the npm install command that installs the deprecated v2 SDK:
locate the line that runs "npm i --save aws-sdk" in tests/test.sh and change it
to install the v3 modular package used in package.json (for example install
"@aws-sdk/client-s3" instead of "aws-sdk"); ensure the test script now uses the
v3 client package name so the installed dependency matches package.json.
- Around line 103-118: The script runs the NodeSource setup_22.x installer with
curl but then downloads and symlinks a Node v12 tarball
(node-v12.22.3-linux-x64.tar.xz), which overrides the intended Node 22 runtime;
either replace the manual download/symlink of node-v12... with a Node 22 tarball
(e.g., node-v22.x-linux-x64.tar.xz) and update the symlink targets, or remove
the wget/tar/ln -s block entirely and instead install the distro package
produced by the setup_22.x step via apt-get install nodejs so the system ends up
running Node 22 as intended. Ensure you update the referenced filenames
(node-v12.22.3-linux-x64 -> node-v22.*-linux-x64) and the symlink targets
(/usr/bin/node /usr/bin/npm /usr/bin/npx) accordingly if you choose the tarball
approach.
| node-version: '18' | ||
| node-version: '22' | ||
| - name: Cypress run | ||
| uses: cypress-io/github-action@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
cypress-io/github-action v4 Cypress 14 compatibility
💡 Result:
cypress-io/github-action@v4 (and all v1–v5) is not compatible with Cypress 14 in practice, because those action majors are unsupported and rely on Node.js 12/16 runtimes. Cypress 14 requires Node.js 18+ to install the cypress npm package, so installs will fail (or be unreliable) under the older action runtime. [1] [2]
What to do instead
- Upgrade your workflow to
cypress-io/github-action@v7(current recommended). It uses a modern Node runtime and is stated compatible with Cypress10+(so includes Cypress 14). [1] - Ensure your install environment meets Cypress 14 requirements (Node 18+ for install; plus OS/library constraints like glibc on Linux). [2]
Sources
- cypress-io/github-action README (Compatibility section) —
v1–v5unsupported;v7recommended; compatible with Cypress10+. [1] - Cypress Migration Guide — Cypress 14 requires Node.js 18+ to install. [2]
🌐 Web query:
cypress-io/github-action latest version 2025
💡 Result:
The latest cypress-io/github-action release in 2025 is v6.10.4, published 2025-11-05. [1]
For most workflows, Cypress recommends pinning to the latest major version tag (currently @v6) rather than a specific patch tag. [2]
Upgrade cypress-io/github-action to v6 or later; v4 is incompatible with Cypress 14.
The v4 action relies on Node.js 12/16, which cannot install Cypress 14 (requires Node.js 18+). This causes install failures and unreliability. Upgrade to cypress-io/github-action@v7 (or @v6), which is compatible with Cypress 10+ and uses a modern Node runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ui-test.yml at line 78, Update the GitHub Actions step
that references the deprecated action version by replacing the uses:
cypress-io/github-action@v4 entry with a modern, compatible tag such as uses:
cypress-io/github-action@v7 (or `@v6`) so the workflow runs on a Node.js 18+
runtime and can install Cypress 14+; ensure any related step inputs/arguments
remain valid for the newer action and run the workflow to confirm no other
breaking changes are needed.
tests/cypress.config.js
Outdated
| chromeWebSecurity: false, | ||
| pageLoadTimeout: 100000, | ||
| defaultCommandTimeout: 100000, | ||
| largeTimeout: 100000, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cypress 14 configuration options timeout settings
💡 Result:
In Cypress v14, timeout-related configuration options (all in milliseconds) include:
defaultCommandTimeout(default 4000) – most DOM commands (likecy.get,cy.contains) retry until this time elapses. [1]pageLoadTimeout(default 60000) –cy.visit(),cy.go(),cy.reload()waiting for the pageloadevent. [1]requestTimeout(default 5000) – waiting for a request to be sent (commonly encountered withcy.wait()). [1]responseTimeout(default shown in docs’ timeout list; configurable) – waiting for a response. [1]execTimeout(default 60000) –cy.exec()timeout. [1]taskTimeout(default 60000) –cy.task()timeout. [1]
Example (cypress.config.ts / cypress.config.js)
const { defineConfig } = require('cypress')
module.exports = defineConfig({
e2e: {
defaultCommandTimeout: 10000,
pageLoadTimeout: 120000,
requestTimeout: 20000,
responseTimeout: 20000,
execTimeout: 120000,
taskTimeout: 120000,
},
})Source: Cypress configuration docs (Timeouts table and writable options). [1]
References
[1] Cypress Docs – Configuration (Timeouts): https://docs.cypress.io/app/references/configuration
🌐 Web query:
Cypress largeTimeout configuration option valid
💡 Result:
No — largeTimeout is not a valid Cypress configuration option (it does not appear in the official list of supported config keys). (docs.cypress.io)
If you’re trying to increase timeouts, use one of Cypress’s supported timeout configs instead, e.g. defaultCommandTimeout, pageLoadTimeout, requestTimeout, responseTimeout, execTimeout, or taskTimeout. (docs.cypress.io)
Citations:
- 1: https://docs.cypress.io/app/references/configuration
- 2: https://docs.cypress.io/app/references/configuration
largeTimeout is not a recognized Cypress configuration option.
This key will be silently ignored by Cypress. Valid timeout options are: defaultCommandTimeout, pageLoadTimeout, requestTimeout, responseTimeout, execTimeout, and taskTimeout. Remove this line to avoid confusion.
Proposed fix
- largeTimeout: 100000,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| largeTimeout: 100000, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cypress.config.js` at line 7, The config contains an unrecognized key
largeTimeout in tests/cypress.config.js which Cypress will ignore; remove the
largeTimeout line (or replace it with the appropriate recognized timeout such as
defaultCommandTimeout, pageLoadTimeout, requestTimeout, responseTimeout,
execTimeout, or taskTimeout) so the timeout setting is applied correctly.
| @@ -27,7 +27,7 @@ | |||
| "devDependencies": { | |||
| "@cypress/browserify-preprocessor": "^3.0.1", | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "index.js" -t f | grep -i pluginRepository: wso2/apim-apps
Length of output: 88
🏁 Script executed:
cat tests/package.json | grep -A 2 -B 2 "@cypress/browserify-preprocessor"Repository: wso2/apim-apps
Length of output: 228
🏁 Script executed:
fd "cypress" -t d | head -20Repository: wso2/apim-apps
Length of output: 72
🏁 Script executed:
cat tests/cypress/plugins/index.jsRepository: wso2/apim-apps
Length of output: 598
🌐 Web query:
@cypress/browserify-preprocessor Cypress 14 compatibility
💡 Result:
@cypress/browserify-preprocessor (latest 3.0.2) is very old/unmaintained (last published ~4 years ago) and is marked deprecated in the ecosystem, but it’s not inherently incompatible with Cypress 14 because Cypress 14 still supports custom bundlers via the file:preprocessor / Preprocessors API. [1] [2] [3]
What does matter for Cypress 14 is mostly your Node/Cypress environment (Cypress 14 requires Node 18+ to install, and bundles Node 20.x), not a Cypress-14-specific peer dependency inside @cypress/browserify-preprocessor (it doesn’t declare one). [4] [1]
Recommendation: If you’re upgrading to Cypress 14, plan to migrate off Browserify (Cypress has deprecated Browserify as a bundler, and major plugins are dropping Browserify support), and use the default webpack-based preprocessor or another maintained bundler (webpack/esbuild). [5] [3]
Sources
[1] https://www.npmjs.com/package/%40cypress/browserify-preprocessor
[2] https://docs.cypress.io/api/node-events/preprocessors-api
[3] https://app.unpkg.com/%40cypress/browserify-preprocessor%403.0.2/files/package.json
[4] https://docs.cypress.io/app/references/migration-guide
[5] https://newreleases.io/project/github/badeball/cypress-cucumber-preprocessor/release/v24.0.0
Migrate @cypress/browserify-preprocessor — it is deprecated and unmaintained.
@cypress/browserify-preprocessor (v3.0.1) has been unmaintained for ~4 years and is marked deprecated in the ecosystem. Cypress deprecated Browserify support in v10 and moved to a webpack-based default bundler. While the package technically still works with Cypress 14 via the file:preprocessor API (see tests/cypress/plugins/index.js), it should be replaced with @cypress/webpack-preprocessor or removed in favor of Cypress's default webpack bundler to ensure long-term compatibility and receive security updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/package.json` at line 28, Remove the deprecated dependency
"@cypress/browserify-preprocessor" from tests/package.json and replace its usage
in tests/cypress/plugins/index.js by switching to
"@cypress/webpack-preprocessor" (or the Cypress default webpack bundler) —
update package.json dependencies to add "@cypress/webpack-preprocessor" if
chosen, modify the plugin setup in tests/cypress/plugins/index.js to instantiate
and use the webpack preprocessor instead of the browserify preprocessor (or
remove the custom file:preprocessor hook to use the default bundler), and run
the test bootstrap to verify the new preprocessor integration.
| @@ -1 +1 @@ | |||
| ./node_modules/.bin/cypress run --headed --spec cypress/integration/admin/08-add-api-categories.spec.js No newline at end of file | |||
| ./node_modules/.bin/cypress run --headed --spec cypress/e2e/admin/08-add-api-categories.cy.js | |||
There was a problem hiding this comment.
Add a shebang line.
This shell script is missing a shebang (#!/bin/bash), which means the interpreter is undefined. This is also flagged by Shellcheck (SC2148).
Proposed fix
+#!/bin/bash
./node_modules/.bin/cypress run --headed --spec cypress/e2e/admin/08-add-api-categories.cy.js📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ./node_modules/.bin/cypress run --headed --spec cypress/e2e/admin/08-add-api-categories.cy.js | |
| #!/bin/bash | |
| ./node_modules/.bin/cypress run --headed --spec cypress/e2e/admin/08-add-api-categories.cy.js |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test.local.sh` at line 1, Add a shebang as the first line of
tests/test.local.sh so the script has a defined shell interpreter (e.g., use
"#!/usr/bin/env bash" or "#!/bin/bash") before the existing command
"./node_modules/.bin/cypress run --headed --spec
cypress/e2e/admin/08-add-api-categories.cy.js"; ensure the shebang is the very
first line and the file is executable.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Cypress testing infrastructure by upgrading from version 9.7.0 to 14.3.2 and updating related dependencies. The changes align the test framework with current best practices for Cypress 10+, which introduced breaking changes to configuration and file structure.
Changes:
- Upgraded Cypress from 9.7.0 to 14.3.2 with new configuration format (cypress.config.js)
- Migrated test file extensions from .spec.js to .cy.js and updated directory structure (integration → e2e)
- Replaced aws-sdk v2 with @aws-sdk/client-s3 v3 and added archiver and nodemailer dependencies
- Updated Node.js version from 12.x to 22.x in test scripts and GitHub Actions workflow
Reviewed changes
Copilot reviewed 6 out of 127 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cypress.config.js | New Cypress 14.x configuration file replacing cypress.json |
| tests/cypress.json | Removed legacy Cypress configuration (migrated to cypress.config.js) |
| tests/package.json | Updated Cypress to 14.3.2, replaced aws-sdk with @aws-sdk/client-s3, added archiver and nodemailer |
| tests/test.sh | Updated Node.js setup from 12.x to 22.x |
| tests/test.local.sh | Updated test spec path to new e2e structure with .cy.js extension |
| tests/cypress/support/e2e.js | New support file for e2e tests (required by Cypress 10+) |
| tests/cypress/e2e/**/*.cy.js | Migrated all test files from integration/.spec.js to e2e/.cy.js |
| .github/workflows/ui-test.yml | Updated Node.js to v22 and test spec pattern to .cy.js |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,43 @@ | |||
| const { defineConfig } = require('cypress') | |||
There was a problem hiding this comment.
Missing copyright header. The file should include the Apache 2.0 license header consistent with other files in the codebase (see tests/package.json or test files for reference).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/cypress.config.js (1)
11-22: Upgradecypress-mochawesome-reporterto v4.0.2 for Node 22 + Cypress 14 compatibility.The current version (v2.4.0) is outdated. Version 4.0.2 (latest) requires Node >=22 and supports Cypress >=6.7.0, making it the correct choice for your setup. The plugin must be explicitly registered in
setupNodeEvents. Updatetests/cypress/plugins/index.jsto include:require('cypress-mochawesome-reporter/plugin')(on);If both mochawesome and junit reports are required, keeping
cypress-multi-reportersas the main reporter is valid, but the plugin registration above must still be added for v4 to function properly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress.config.js` around lines 11 - 22, Upgrade the cypress-mochawesome-reporter dependency to v4.0.2 in package.json and keep cypress-multi-reporters as the main reporter in the reporter configuration; then register the mochawesome plugin inside your Cypress node event setup by adding the plugin require call in the setupNodeEvents function (in tests/cypress/plugins/index.js) so that require('cypress-mochawesome-reporter/plugin')(on) is invoked when setupNodeEvents receives the on argument; ensure the reporterOptions remain but that the plugin registration is present so v4 works with Node 22 and Cypress 14.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cypress.config.js`:
- Around line 34-38: The excludeSpecPattern entry '**/*.cy.skip.js' is a dead
pattern because specPattern only discovers files ending exactly in
.cy.{js,jsx,ts,tsx}; remove the redundant '**/*.cy.skip.js' from the
excludeSpecPattern array in tests/cypress.config.js (or alternatively
standardize skipped-test filenames to match the discovered suffixes, e.g.,
rename to *.cy.js.skip and adjust specPattern accordingly), ensuring you update
the excludeSpecPattern array and any test filenames consistently; look for the
excludeSpecPattern and specPattern identifiers to make the change.
- Around line 12-22: Move the misplaced inlineAssets key from the top level of
reporterOptions into the cypressMochawesomeReporterReporterOptions block so the
cypress-mochawesome-reporter actually receives it; update the reporterOptions
object (referencing reporterOptions, inlineAssets and
cypressMochawesomeReporterReporterOptions) to nest inlineAssets under
cypressMochawesomeReporterReporterOptions rather than at the root.
- Line 10: Remove the obsolete experimentalInteractiveRunEvents option from the
Cypress config (tests/cypress.config.js); this flag is no longer recognized in
Cypress 14 and is redundant because setupNodeEvents already provides the
before:run/after:run/before:spec/after:spec behavior—delete the
experimentalInteractiveRunEvents entry and ensure any event handlers are
implemented inside the setupNodeEvents function.
---
Duplicate comments:
In `@tests/cypress.config.js`:
- Line 7: The config object in tests/cypress.config.js uses an invalid key
largeTimeout; remove or replace it with the supported Cypress timeout keys
(e.g., defaultCommandTimeout, pageLoadTimeout, requestTimeout, responseTimeout)
where appropriate. Locate the config object that contains largeTimeout and
either delete that property or map its numeric value to the correct key(s) such
as defaultCommandTimeout or pageLoadTimeout depending on what the timeout was
intended to control.
---
Nitpick comments:
In `@tests/cypress.config.js`:
- Around line 11-22: Upgrade the cypress-mochawesome-reporter dependency to
v4.0.2 in package.json and keep cypress-multi-reporters as the main reporter in
the reporter configuration; then register the mochawesome plugin inside your
Cypress node event setup by adding the plugin require call in the
setupNodeEvents function (in tests/cypress/plugins/index.js) so that
require('cypress-mochawesome-reporter/plugin')(on) is invoked when
setupNodeEvents receives the on argument; ensure the reporterOptions remain but
that the plugin registration is present so v4 works with Node 22 and Cypress 14.
| reporterOptions: { | ||
| inlineAssets: true, | ||
| reporterEnabled: 'cypress-mochawesome-reporter, mocha-junit-reporter', | ||
| mochaJunitReporterReporterOptions: { | ||
| mochaFile: 'cypress/reports/junit/results-[hash].xml', | ||
| }, | ||
| cypressMochawesomeReporterReporterOptions: { | ||
| charts: true, | ||
| reportPageTitle: 'custom-title', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
inlineAssets is misplaced at the top level of reporterOptions and will be silently ignored by cypress-mochawesome-reporter.
When using cypress-multi-reporters, each sub-reporter's options must be nested under its corresponding <camelCasedPackageName>ReporterOptions key. inlineAssets is a cypress-mochawesome-reporter option and belongs inside cypressMochawesomeReporterReporterOptions, not at the root of reporterOptions.
🐛 Proposed fix
reporterOptions: {
- inlineAssets: true,
reporterEnabled: 'cypress-mochawesome-reporter, mocha-junit-reporter',
mochaJunitReporterReporterOptions: {
mochaFile: 'cypress/reports/junit/results-[hash].xml',
},
cypressMochawesomeReporterReporterOptions: {
charts: true,
reportPageTitle: 'custom-title',
+ inlineAssets: true,
},
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cypress.config.js` around lines 12 - 22, Move the misplaced
inlineAssets key from the top level of reporterOptions into the
cypressMochawesomeReporterReporterOptions block so the
cypress-mochawesome-reporter actually receives it; update the reporterOptions
object (referencing reporterOptions, inlineAssets and
cypressMochawesomeReporterReporterOptions) to nest inlineAssets under
cypressMochawesomeReporterReporterOptions rather than at the root.
| excludeSpecPattern: [ | ||
| '**/externalDevPortals/**/*.cy.js', | ||
| '**/06-solace-broker-integration.cy.js', | ||
| '**/*.cy.skip.js', | ||
| ], |
There was a problem hiding this comment.
'**/*.cy.skip.js' in excludeSpecPattern is a dead pattern and will never match any discovered spec.
The specPattern is cypress/e2e/**/*.cy.{js,jsx,ts,tsx}, which only discovers files whose name ends in exactly .cy.js, .cy.jsx, .cy.ts, or .cy.tsx. A file named foo.cy.skip.js ends in .cy.skip.js, which does not satisfy any of those suffixes, so it is never picked up by the spec scanner in the first place. The exclude rule is therefore a no-op and should be removed (or the convention should be reconsidered—e.g., renaming skipped tests to *.cy.js.skip instead).
♻️ Proposed fix
excludeSpecPattern: [
'**/externalDevPortals/**/*.cy.js',
'**/06-solace-broker-integration.cy.js',
- '**/*.cy.skip.js',
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| excludeSpecPattern: [ | |
| '**/externalDevPortals/**/*.cy.js', | |
| '**/06-solace-broker-integration.cy.js', | |
| '**/*.cy.skip.js', | |
| ], | |
| excludeSpecPattern: [ | |
| '**/externalDevPortals/**/*.cy.js', | |
| '**/06-solace-broker-integration.cy.js', | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cypress.config.js` around lines 34 - 38, The excludeSpecPattern entry
'**/*.cy.skip.js' is a dead pattern because specPattern only discovers files
ending exactly in .cy.{js,jsx,ts,tsx}; remove the redundant '**/*.cy.skip.js'
from the excludeSpecPattern array in tests/cypress.config.js (or alternatively
standardize skipped-test filenames to match the discovered suffixes, e.g.,
rename to *.cy.js.skip and adjust specPattern accordingly), ensuring you update
the excludeSpecPattern array and any test filenames consistently; look for the
excludeSpecPattern and specPattern identifiers to make the change.
…ariable for largeTimeout
- Updated all instances of Cypress.config().largeTimeout to Cypress.env('largeTimeout') across multiple test files and support commands.
- Ensured consistent timeout handling for various Cypress commands to improve test reliability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tests/cypress/e2e/publisher/011-lifecycle/04-depricate-old-versions.cy.js (1)
52-52:⚠️ Potential issue | 🟡 MinorRemove
it.onlybefore merging — debugging artifact.
it.onlycauses all otheritblocks in this file to be skipped if more tests are added later, and is a leftover debugging marker that should not be committed to the main branch.🐛 Proposed fix
- it.only("Depricate old versions of api before publishing", () => { + it("Depricate old versions of api before publishing", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/011-lifecycle/04-depricate-old-versions.cy.js` at line 52, The test uses a debugging marker — change the focused test from it.only("Depricate old versions of api before publishing", ...) to it("Depricate old versions of api before publishing", ...) (i.e., remove the .only) in the file where that spec is defined; also scan for any other occurrences of .only in the same test file (or across tests) and remove them so the full suite runs normally, then run the Cypress tests to verify no accidental focusing remains.tests/cypress/e2e/publisher/014-comments/01-add-nested-comments-to-api.cy.js (1)
57-66:⚠️ Potential issue | 🟠 MajorPre-existing:
cy.interceptregistered afterclick— network request may be missedThe intercept alias
@deleteCommentis set up inside the.within()callback after.click()has already been enqueued:cy.get("#comment-list").within(() => { cy.get("button").contains("Delete").click(); // ← click queued first cy.intercept("DELETE", "**/comments/**").as("deleteComment"); // ← intercept queued second }); cy.wait("@deleteComment", { timeout: 30000 }); // may never be satisfiedCypress executes the enqueued
clickbefore theinterceptis registered, so if the DELETE request fires in that window,cy.wait("@deleteComment")will never resolve and the test will time out. The intercept must be set up before the action that triggers the request.🐛 Proposed fix — move intercept before click
+ cy.intercept("DELETE", "**/comments/**").as("deleteComment"); cy.get("#comment-list").within(() => { cy.get("button").contains("Delete").click(); - cy.intercept("DELETE", "**/comments/**").as("deleteComment"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/014-comments/01-add-nested-comments-to-api.cy.js` around lines 57 - 66, The intercept for the DELETE request is registered after the click that triggers it, so cy.wait("@deleteComment") can time out; move the cy.intercept("DELETE", "**/comments/**").as("deleteComment") to run before the action that triggers the request (i.e., before calling .click()), register it outside or before cy.get("#comment-list").within(), then perform cy.get(...).contains("Delete").click() and keep the cy.wait("@deleteComment", { timeout: 30000 }) to wait for the aliased request.tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js (1)
115-115:⚠️ Potential issue | 🟡 MinorTypo in comment: "verions" → "versions".
✏️ Proposed fix
- // delete api product verions + // delete api product versions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js` at line 115, Fix the typo in the inline comment "// delete api product verions" by changing "verions" to "versions" so it reads "// delete api product versions"; locate this comment in the test file around the API product deletion logic and update only the comment text (no code changes).tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js (1)
158-163:⚠️ Potential issue | 🟡 MinorRemove
it.only— should not be committed to the main branch.Both tests use
.only, which will silently skip any futureit(...)tests added to this describe block in CI, reducing coverage without an obvious failure signal.Additionally, this pattern appears systematically across the entire test suite (100+ instances). This suggests test files were left with debugging
.onlymarkers committed—these should be cleaned up project-wide.🛠️ Proposed fix
- it.only("Add key manager - super admin", () => { + it("Add key manager - super admin", () => { addKeyManager(carbonUsername, carbonPassword); }); - it.only("Add key manager - tenant user", () => { + it("Add key manager - tenant user", () => { addKeyManager(Utils.getTenantUser(carbonUsername, tenant), carbonPassword); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js` around lines 158 - 163, Remove the accidental test-only markers: replace both occurrences of "it.only" used in the tests titled "Add key manager - super admin" and "Add key manager - tenant user" with regular "it" so these specs don't suppress other tests; search for remaining ".only" usages across the test suite (the failing pattern appears widely) and remove them as well (keep test names and calls to addKeyManager and Utils.getTenantUser intact).tests/cypress/e2e/devportal/000-general/01-anonymous-user-view-public-apis.cy.js (1)
52-61:⚠️ Potential issue | 🟡 Minor
Cypress.$is jQuery and silently ignores thetimeoutoption.
Cypress.$is a synchronous jQuery wrapper and its constructor has no concept of retries or timeouts — the{ timeout: Cypress.env('largeTimeout') }third argument passed on line 53 is silently discarded by jQuery, making it misleading dead code regardless of what value it holds.The fix is to remove it from the
Cypress.$call. If a true timeout/retry loop is needed here, the correct approach is acy.get()with.should()or a recursive custom command.🐛 Proposed fix — remove the no-op timeout option
-let $apis = Cypress.$('a', apiName, { timeout: Cypress.env('largeTimeout') }); +let $apis = Cypress.$('a', apiName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/devportal/000-general/01-anonymous-user-view-public-apis.cy.js` around lines 52 - 61, The use of Cypress.$('a', apiName, { timeout: Cypress.env('largeTimeout') }) is incorrect because Cypress.$ is a synchronous jQuery call that ignores the timeout option; remove the third argument so the call becomes Cypress.$('a', apiName) and rely on proper Cypress retry semantics instead — either replace the polling loop with cy.get(selector).should('contain', apiName) or implement a recursive/custom cy command if you need explicit retry behavior; update references to attemptCount/remainingAttempts accordingly and keep cy.reload() if you still want to refresh between attempts.
🧹 Nitpick comments (8)
tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js (1)
109-111: Incomplete migration: hardcoded30000timeouts should useCypress.env('largeTimeout').Lines 109 and 111 still use the magic number
30000while all other timeout usages in this file were migrated toCypress.env('largeTimeout'). This creates an inconsistency — iflargeTimeoutis configured to a different value (e.g., for slower environments), these two assertions will behave differently from the rest of the test.♻️ Proposed fix
- cy.wait('@apiGet', { timeout: 30000 }).then(() => { + cy.wait('@apiGet', { timeout: Cypress.env('largeTimeout') }).then(() => { // validate - cy.get('#itest-api-name-version', { timeout: 30000 }).should('be.visible'); + cy.get('#itest-api-name-version', { timeout: Cypress.env('largeTimeout') }).should('be.visible');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js` around lines 109 - 111, Replace the hardcoded timeout 30000 in the cy.wait('@apiGet', { timeout: 30000 }) and cy.get('#itest-api-name-version', { timeout: 30000 }) calls with the shared config value Cypress.env('largeTimeout'); locate the two occurrences around the cy.wait('@apiGet') and the subsequent cy.get('#itest-api-name-version') and change their timeout option to use Cypress.env('largeTimeout') so the test uses the consistent largeTimeout setting.tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js (2)
56-56: Consider usingCypress.env('largeTimeout')for consistency.The
3000ms timeout oncy.wait('@importConfig')is the only remaining hard-coded timeout in this file after the migration. Replacing it aligns with the PR-widelargeTimeoutpattern and avoids a potential flake if the endpoint is slow in CI.♻️ Proposed fix
- cy.wait('@importConfig', { timeout: 3000 }).then(() => { + cy.wait('@importConfig', { timeout: Cypress.env('largeTimeout') }).then(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js` at line 56, Replace the hard-coded 3000ms timeout in the cy.wait call for the '@importConfig' route with the PR-wide env variable: change the timeout option in the call that matches "cy.wait('@importConfig', { timeout: 3000 })" to use Cypress.env('largeTimeout') so it becomes consistent with other tests and avoids a flaky hard-coded value.
87-91: Replacecy.wait(1000)with a deterministic assertion.Unconditional waits make tests slower and flakier. The wait at line 88 follows
.click()on#keymanager-add— the next assertion on line 91 (cy.get('td > div').contains(km).should('exist')) is already retryable and serves as a natural gate.♻️ Proposed fix
cy.get('#keymanager-add').contains('Add').click(); - cy.wait(1000); // validating cy.get('td > div').contains(km).should('exist');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js` around lines 87 - 91, Remove the unconditional cy.wait(1000) after clicking '#keymanager-add' and replace it with a deterministic assertion: either remove it entirely and rely on the retryable assertion cy.get('td > div').contains(km).should('exist'), or add an explicit assertion that the add action completed (e.g., modal or spinner is gone, or '#keymanager-add' is enabled/hidden) before asserting the presence of km; update the test around the '#keymanager-add' click and the cy.get('td > div').contains(km).should('exist') assertion accordingly.tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.js (1)
40-40: Remove stale commented-out assertion.The inline comment acknowledges this assertion "haven't worked". Carrying dead code through a major version migration adds noise and leaves false signals for future readers. Either fix the assertion or delete it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.js` at line 40, Remove the stale commented-out assertion line "//cy.get('div[role="status"]', {timeout: Cypress.env('largeTimeout')}).should('have.text', 'API updated successfully');" from the test; if you want the check preserved instead of deleted, re-enable it by fixing the selector/timeout and expectation (use cy.get('div[role="status"]', { timeout: Cypress.env('largeTimeout') }).should('contain.text', 'API updated successfully') or target the correct status element) so the assertion is reliable, otherwise delete the commented line to avoid dead code.tests/cypress/support/commands.js (1)
439-440:timeouton.click({ force: true })is a no-op.
Cypress.click()is not retried because it could potentially change the state of the application under test; after the click occurs, Cypress will also not re-run any queries before.click(). Thetimeoutoption on action commands controls how long Cypress waits for actionability checks before executing the click. Withforce: true, all actionability checks are bypassed immediately, so the large timeout has zero effect. The intent (waiting for the button to be available) requires the timeout on the preceding.get():♻️ Proposed fix
-cy.get('[data-testid="itest-create-graphql-api-button"]',) - .click({ force: true, timeout: Cypress.env('largeTimeout') }); +cy.get('[data-testid="itest-create-graphql-api-button"]', { timeout: Cypress.env('largeTimeout') }) + .click({ force: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/support/commands.js` around lines 439 - 440, The click call is using force:true which bypasses actionability checks so the timeout there is a no-op; update the chain so the timeout is applied on cy.get('[data-testid="itest-create-graphql-api-button"]') (e.g., cy.get(..., { timeout: Cypress.env('largeTimeout') })) and remove the redundant timeout option from .click({ force: true }) (and tidy the trailing comma) so Cypress waits for the element before forcing the click.tests/cypress/e2e/devportal/002-subscriptions/02-subscribe-via-wizard.cy.js (1)
54-63: Consider chaining.click()directly rather than issuing a secondcy.get().The pattern on lines 54-55, 57-58, and 62-63 queries the same element twice — once with the large timeout (but no assertion/action) and again without it to
.click(). The timeout guard on the first call doesn't carry over to the second, and the redundant query adds noise. The idiomatic form is a single chained call:♻️ Proposed refactor
- cy.get('#wizard-next-1-btn', {timeout: Cypress.env('largeTimeout')}) - cy.get('#wizard-next-1-btn').click(); + cy.get('#wizard-next-1-btn', {timeout: Cypress.env('largeTimeout')}).click(); - cy.get('#wizard-next-2-btn', {timeout: Cypress.env('largeTimeout')}); - cy.get('#wizard-next-2-btn').click(); + cy.get('#wizard-next-2-btn', {timeout: Cypress.env('largeTimeout')}).click(); cy.intercept('GET','**/oauth-keys').as('oauthKeys'); cy.wait('@oauthKeys', {timeout: Cypress.env('largeTimeout')}).then(() => { - cy.get('#wizard-next-3-btn', {timeout: Cypress.env('largeTimeout')}); - cy.get('#wizard-next-3-btn').click(); + cy.get('#wizard-next-3-btn', {timeout: Cypress.env('largeTimeout')}).click(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/devportal/002-subscriptions/02-subscribe-via-wizard.cy.js` around lines 54 - 63, The test issues redundant element queries: each '#wizard-next-1-btn', '#wizard-next-2-btn' and '#wizard-next-3-btn' is fetched once with a timeout and then fetched again to click; instead chain the click onto the initial get so the timeout applies to the action (e.g., replace cy.get('#wizard-next-1-btn', {timeout: Cypress.env('largeTimeout')}); cy.get('#wizard-next-1-btn').click(); with a single cy.get('#wizard-next-1-btn', {timeout: Cypress.env('largeTimeout')}).click()), and do the same for the other two selectors (including the one inside the oauthKeys wait callback) to remove duplicate queries and ensure the timeout is honored.tests/cypress/e2e/publisher/012-documents/02-view-generated-document-not-rest.cy.js (1)
59-68: TheCypress.env('largeTimeout')change at line 63 is inside a commented-outit.onlyblock — it has no effect.The entire second test case (
"Viewing generated document in devportal") is wrapped in a/* ... */block comment and will never execute. Consider either restoring the test (and removing the comment wrapper) or deleting it outright to avoid accumulating dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/012-documents/02-view-generated-document-not-rest.cy.js` around lines 59 - 68, The commented-out test "Viewing generated document in devportal" (the it.only block referencing Cypress.env('largeTimeout') and selectors like `[area-label="Go to ${genApiName}"]`, '#left-menu-documents', and '#apim_elements') is dead code and has no effect; either restore it by removing the surrounding /* ... */ so the test runs (keep the it.only only if you intend to run it exclusively and ensure login helpers cy.loginToDevportal/cy.loginToPublisher and Utils.getUserInfo() are correct) or delete the entire commented block to remove stale code and avoid confusion.tests/cypress/e2e/publisher/013-api-product/06-download-api-product.cy.js (1)
52-52: Barecy.get()used as an implicit wait — add an explicit assertion for clarity.Lines 52, 64, and 93 call
cy.get(selector, {timeout: ...})and discard the result with no assertion. While Cypress will retry until the element is found (implicitly asserting existence), the intent is not obvious to the reader and the pattern makes it easy to forget to add a meaningful assertion later.♻️ Suggested refactor (apply to lines 52, 64, and 93 identically)
-cy.get('#itest-id-apiversion-input', { timeout: Cypress.env('largeTimeout') }); +cy.get('#itest-id-apiversion-input', { timeout: Cypress.env('largeTimeout') }).should('exist');-cy.get('#itest-api-name-version', { timeout: Cypress.env('largeTimeout') }); +cy.get('#itest-api-name-version', { timeout: Cypress.env('largeTimeout') }).should('be.visible');Also applies to: 64-64, 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/publisher/013-api-product/06-download-api-product.cy.js` at line 52, The three bare cy.get(...) calls in tests/cypress/e2e/publisher/013-api-product/06-download-api-product.cy.js (notably the selector '#itest-id-apiversion-input' and the similar calls at the other two locations) are being used as implicit waits with no assertion; replace each lone cy.get(selector, { timeout: Cypress.env('largeTimeout') }) with an explicit assertion such as cy.get(selector, { timeout: Cypress.env('largeTimeout') }).should('be.visible') (or .should('exist') if visibility is not required) so the intent is clear and the test fails meaningfully when the element is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.js`:
- Line 43: The test is passing a spurious `{timeout: ...}` object to `.should()`
on the cy.get('#http-transport', {timeout: Cypress.env('largeTimeout')}) call;
remove the second `{timeout: ...}` argument so the assertion reads
`.should('not.be.checked')` (the timeout stays on `cy.get` and will be
inherited), i.e., update the statement using the '#http-transport' selector to
drop the extra options object from `.should()`.
In
`@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js`:
- Line 43: Replace all hardcoded timeout objects `{ timeout: 30000 }` in the
Cypress test with the environment value `{ timeout: Cypress.env('largeTimeout')
}` so the file consistently uses the centralized timeout (update occurrences
around the clicks/visits at the same spots as the review: the instances near the
selectors used in this spec), and correct the typo "verions" to "versions"
(search for the string "verions" in this test and change it to "versions");
specifically update the cy.visit/cy.get calls that currently pass `{ timeout:
30000 }` to use `Cypress.env('largeTimeout')` and fix the misspelled label.
---
Outside diff comments:
In `@tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js`:
- Around line 158-163: Remove the accidental test-only markers: replace both
occurrences of "it.only" used in the tests titled "Add key manager - super
admin" and "Add key manager - tenant user" with regular "it" so these specs
don't suppress other tests; search for remaining ".only" usages across the test
suite (the failing pattern appears widely) and remove them as well (keep test
names and calls to addKeyManager and Utils.getTenantUser intact).
In
`@tests/cypress/e2e/devportal/000-general/01-anonymous-user-view-public-apis.cy.js`:
- Around line 52-61: The use of Cypress.$('a', apiName, { timeout:
Cypress.env('largeTimeout') }) is incorrect because Cypress.$ is a synchronous
jQuery call that ignores the timeout option; remove the third argument so the
call becomes Cypress.$('a', apiName) and rely on proper Cypress retry semantics
instead — either replace the polling loop with
cy.get(selector).should('contain', apiName) or implement a recursive/custom cy
command if you need explicit retry behavior; update references to
attemptCount/remainingAttempts accordingly and keep cy.reload() if you still
want to refresh between attempts.
In `@tests/cypress/e2e/publisher/011-lifecycle/04-depricate-old-versions.cy.js`:
- Line 52: The test uses a debugging marker — change the focused test from
it.only("Depricate old versions of api before publishing", ...) to it("Depricate
old versions of api before publishing", ...) (i.e., remove the .only) in the
file where that spec is defined; also scan for any other occurrences of .only in
the same test file (or across tests) and remove them so the full suite runs
normally, then run the Cypress tests to verify no accidental focusing remains.
In
`@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js`:
- Line 115: Fix the typo in the inline comment "// delete api product verions"
by changing "verions" to "versions" so it reads "// delete api product
versions"; locate this comment in the test file around the API product deletion
logic and update only the comment text (no code changes).
In
`@tests/cypress/e2e/publisher/014-comments/01-add-nested-comments-to-api.cy.js`:
- Around line 57-66: The intercept for the DELETE request is registered after
the click that triggers it, so cy.wait("@deleteComment") can time out; move the
cy.intercept("DELETE", "**/comments/**").as("deleteComment") to run before the
action that triggers the request (i.e., before calling .click()), register it
outside or before cy.get("#comment-list").within(), then perform
cy.get(...).contains("Delete").click() and keep the cy.wait("@deleteComment", {
timeout: 30000 }) to wait for the aliased request.
---
Duplicate comments:
In `@tests/cypress/e2e/admin/05-add-custom-throttle-policies.cy.js`:
- Around line 34-49: The test uses Cypress.env('largeTimeout') in multiple
places (e.g., the cy.get('[data-testid="Custom Policies-child-link"]', {timeout:
Cypress.env('largeTimeout')}), cy.wait('@getCustomPolicies', {timeout:
Cypress.env('largeTimeout')})) which can be undefined; change those calls to use
a safe fallback when the env var is missing (e.g., use a default numeric timeout
or read Cypress.config('defaultCommandTimeout') as a fallback) so the timeout
option is always a number and the test cannot fail with an undefined timeout.
In
`@tests/cypress/e2e/devportal/003-test/00-tryout-api-invocations-from-swagger-console.cy.js`:
- Line 83: The cy.wait call using an undefined environment variable risks
passing undefined as timeout; update the cy.wait('@genToken', { timeout:
Cypress.env('largeTimeout') }) usage to resolve a safe numeric timeout (e.g.,
read Cypress.env('largeTimeout') and fall back to a known default or use
Cypress.config('defaultCommandTimeout')) before passing it to cy.wait so the
'@genToken' wait always receives a valid number; modify the cy.wait invocation
in the test file to compute a safeTimeout variable and pass that instead.
In
`@tests/cypress/e2e/publisher/002-api-resources/02-add-assign-global-scopes-for-api.cy.js`:
- Line 37: The cy.visit(...) calls in 02-add-assign-global-scopes-for-api.cy.js
are passing Cypress.env('largeTimeout') directly which can be undefined; update
each cy.visit usage (the three calls that set the timeout option) to use a safe
fallback so the timeout is always a number—e.g., replace
Cypress.env('largeTimeout') with a defensive expression like
Cypress.env('largeTimeout') || <reasonableDefaultMillis> or read from
Cypress.config('defaultCommandTimeout')—so the timeout option is never undefined
for the cy.visit calls.
In
`@tests/cypress/e2e/publisher/004-endpoints/01-add-soap-endpoints-production-sandbox.cy.js`:
- Around line 32-46: The tests call Cypress.env('largeTimeout') for several
get() calls (e.g., selectors '#itest-api-details-api-config-acc',
'[data-testid="http/soapendpoint-add-btn"]', '#endpoint-save-btn') which risks
an undefined timeout; replace these uses with a safe timeout fallback (e.g., use
Cypress.env('largeTimeout') || Cypress.config('defaultCommandTimeout') or a
defined numeric constant) and apply the fallback to every get() and click() that
currently passes Cypress.env('largeTimeout') so the commands always have a valid
timeout.
In
`@tests/cypress/e2e/publisher/006-subscriptions/01-subscribtion-blocking.cy.js`:
- Around line 53-83: The test uses Cypress.env('largeTimeout') in multiple
cy.get calls (e.g., the selectors '#application-subscribe',
'#itest-api-details-portal-config-acc', and the afterEach selector
`#delete-${appName}-btn`) which risks an undefined timeout; ensure a safe
timeout is always provided by either reading a validated env variable (and
throwing or setting a default if missing) or by using a fallback/default timeout
value when calling Cypress.env('largeTimeout'), and update all occurrences in
this file (the cy.get calls and any other waits that rely on that env) to use
that validated/fallback timeout.
In
`@tests/cypress/e2e/publisher/020-prototyped-apis/00-try-out-resources-without-key-when-security-disabled.cy.js`:
- Around line 93-94: The selector uses a misspelled attribute and incorrect
Cypress chaining: replace the `area-label` attribute with `aria-label` and
change the second `.get()` to `.find()` when targeting the element using
`apiName` (selectors involving `table > tbody > tr` and the template
`[area-label="Go to ${apiName}"]`); update both the assertion and the click so
they query `table > tbody > tr` then `.find(\`[aria-label="Go to
${apiName}"]\`)` before `.should('contain.text', 'PRE-RELEASED')` and
`.click()`.
In
`@tests/cypress/e2e/publisher/020-prototyped-apis/01-cannot-try-out-resources-without-key-when-security-enabled.cy.js`:
- Around line 81-82: Fix the selector typo and chaining: in the test lines using
cy.get('table > tbody > tr', ...).get(`[area-label="Go to ${apiName}"]`) change
the attribute to `aria-label` and replace the second .get() with .find() so the
call becomes cy.get('table > tbody > tr', ...).find(`[aria-label="Go to
${apiName}"]`) for both the assertion and the click; update both occurrences
that reference the `${apiName}` aria selector.
In `@tests/cypress/e2e/publisher/022-gateway-policies/00-gateway-policy.cy.js`:
- Around line 31-110: The tests use Cypress.env('largeTimeout') in multiple
places (e.g., cy.visit(..., { timeout: Cypress.env('largeTimeout') }),
cy.contains(..., { timeout: Cypress.env('largeTimeout') })) which can be
undefined; update the spec to use a safe timeout value or a fallback (for
example read a defined constant like largeTimeout from top of the file or use
Cypress.config('defaultCommandTimeout') / a numeric literal) and replace all
occurrences of Cypress.env('largeTimeout') so timeouts are guaranteed; search
for cy.visit, cy.contains, cy.get, and cy.get(..., { timeout:
Cypress.env('largeTimeout') }) and replace them with the chosen reliable timeout
reference.
---
Nitpick comments:
In `@tests/cypress/e2e/admin/09-add-edit-delete-km.cy.js`:
- Line 56: Replace the hard-coded 3000ms timeout in the cy.wait call for the
'@importConfig' route with the PR-wide env variable: change the timeout option
in the call that matches "cy.wait('@importConfig', { timeout: 3000 })" to use
Cypress.env('largeTimeout') so it becomes consistent with other tests and avoids
a flaky hard-coded value.
- Around line 87-91: Remove the unconditional cy.wait(1000) after clicking
'#keymanager-add' and replace it with a deterministic assertion: either remove
it entirely and rely on the retryable assertion cy.get('td >
div').contains(km).should('exist'), or add an explicit assertion that the add
action completed (e.g., modal or spinner is gone, or '#keymanager-add' is
enabled/hidden) before asserting the presence of km; update the test around the
'#keymanager-add' click and the cy.get('td > div').contains(km).should('exist')
assertion accordingly.
In `@tests/cypress/e2e/devportal/002-subscriptions/02-subscribe-via-wizard.cy.js`:
- Around line 54-63: The test issues redundant element queries: each
'#wizard-next-1-btn', '#wizard-next-2-btn' and '#wizard-next-3-btn' is fetched
once with a timeout and then fetched again to click; instead chain the click
onto the initial get so the timeout applies to the action (e.g., replace
cy.get('#wizard-next-1-btn', {timeout: Cypress.env('largeTimeout')});
cy.get('#wizard-next-1-btn').click(); with a single cy.get('#wizard-next-1-btn',
{timeout: Cypress.env('largeTimeout')}).click()), and do the same for the other
two selectors (including the one inside the oauthKeys wait callback) to remove
duplicate queries and ensure the timeout is honored.
In
`@tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.js`:
- Line 40: Remove the stale commented-out assertion line
"//cy.get('div[role="status"]', {timeout:
Cypress.env('largeTimeout')}).should('have.text', 'API updated successfully');"
from the test; if you want the check preserved instead of deleted, re-enable it
by fixing the selector/timeout and expectation (use cy.get('div[role="status"]',
{ timeout: Cypress.env('largeTimeout') }).should('contain.text', 'API updated
successfully') or target the correct status element) so the assertion is
reliable, otherwise delete the commented line to avoid dead code.
In
`@tests/cypress/e2e/publisher/012-documents/02-view-generated-document-not-rest.cy.js`:
- Around line 59-68: The commented-out test "Viewing generated document in
devportal" (the it.only block referencing Cypress.env('largeTimeout') and
selectors like `[area-label="Go to ${genApiName}"]`, '#left-menu-documents', and
'#apim_elements') is dead code and has no effect; either restore it by removing
the surrounding /* ... */ so the test runs (keep the it.only only if you intend
to run it exclusively and ensure login helpers
cy.loginToDevportal/cy.loginToPublisher and Utils.getUserInfo() are correct) or
delete the entire commented block to remove stale code and avoid confusion.
In
`@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js`:
- Around line 109-111: Replace the hardcoded timeout 30000 in the
cy.wait('@apiGet', { timeout: 30000 }) and cy.get('#itest-api-name-version', {
timeout: 30000 }) calls with the shared config value
Cypress.env('largeTimeout'); locate the two occurrences around the
cy.wait('@apiGet') and the subsequent cy.get('#itest-api-name-version') and
change their timeout option to use Cypress.env('largeTimeout') so the test uses
the consistent largeTimeout setting.
In `@tests/cypress/e2e/publisher/013-api-product/06-download-api-product.cy.js`:
- Line 52: The three bare cy.get(...) calls in
tests/cypress/e2e/publisher/013-api-product/06-download-api-product.cy.js
(notably the selector '#itest-id-apiversion-input' and the similar calls at the
other two locations) are being used as implicit waits with no assertion; replace
each lone cy.get(selector, { timeout: Cypress.env('largeTimeout') }) with an
explicit assertion such as cy.get(selector, { timeout:
Cypress.env('largeTimeout') }).should('be.visible') (or .should('exist') if
visibility is not required) so the intent is clear and the test fails
meaningfully when the element is missing.
In `@tests/cypress/support/commands.js`:
- Around line 439-440: The click call is using force:true which bypasses
actionability checks so the timeout there is a no-op; update the chain so the
timeout is applied on cy.get('[data-testid="itest-create-graphql-api-button"]')
(e.g., cy.get(..., { timeout: Cypress.env('largeTimeout') })) and remove the
redundant timeout option from .click({ force: true }) (and tidy the trailing
comma) so Cypress waits for the element before forcing the click.
| cy.contains('API updated successfully', {timeout: Cypress.env('largeTimeout')}); | ||
| cy.get('#transportLevel').click({force:true}); | ||
| cy.get('#http-transport', {timeout: Cypress.config().largeTimeout}).should('not.be.checked', {timeout: Cypress.config().largeTimeout}); | ||
| cy.get('#http-transport', {timeout: Cypress.env('largeTimeout')}).should('not.be.checked', {timeout: Cypress.env('largeTimeout')}); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. First, find the cypress.config.js file
find . -name "cypress.config.js" -o -name "cypress.config.ts" | head -5Repository: wso2/apim-apps
Length of output: 83
🏁 Script executed:
#!/bin/bash
# 2. Search for the pattern mentioned in the review to find similar issues
rg -n --type js "\.should\(['\"][^'\"]+['\"],\s*\{timeout" tests/Repository: wso2/apim-apps
Length of output: 287
🏁 Script executed:
#!/bin/bash
# 3. Also check for similar patterns with Cypress.env() in timeout contexts
rg -n --type js "Cypress\.env\('largeTimeout'\)" tests/ | head -20Repository: wso2/apim-apps
Length of output: 2624
🏁 Script executed:
#!/bin/bash
# 4. Check the actual line 43 in the file mentioned
sed -n '40,45p' tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.jsRepository: wso2/apim-apps
Length of output: 571
🏁 Script executed:
#!/bin/bash
# Read the cypress.config.js to verify largeTimeout is defined
cat tests/cypress.config.jsRepository: wso2/apim-apps
Length of output: 1185
🏁 Script executed:
#!/bin/bash
# Also check for largeTimeout in environment setup or other config files
rg -n "largeTimeout" tests/ --type js --type ts | grep -E "(env|config)" | head -10Repository: wso2/apim-apps
Length of output: 1457
🏁 Script executed:
#!/bin/bash
# Check the Cypress version in package.json
grep -A2 -B2 '"cypress"' tests/package.json || grep -A2 -B2 '"cypress"' package.json | head -10Repository: wso2/apim-apps
Length of output: 265
🌐 Web query:
Cypress.env() vs cy.env() Cypress 14 deprecation
💡 Result:
In Cypress 14 (released Jan 16, 2025), there is no Cypress.env() → cy.env() deprecation. The deprecation starts in Cypress 15.10.0 (released Feb 3, 2026), where Cypress.env() is deprecated and cy.env() is introduced. [1][2]
Cypress.env() vs cy.env()
Cypress.env()(deprecated in 15.10.0+): synchronous, but it hydrates all configured env vars into the browser context, which can unintentionally expose secrets. [2]cy.env()(added in 15.10.0+): a Cypress command (async/chainable) that only exposes the variables you explicitly request and avoids automatically serializing all env into browser state (safer for secrets). [3]
What to use now
- Use
cy.env()for sensitive values/secrets you want to keep tightly scoped to what’s requested. [3] - Use
Cypress.expose()for non-sensitive/public config you’re okay with being available in the browser context. [1][3]
Migration/enforcement
- Cypress provides a migration guide for replacing
Cypress.env()withcy.env()/Cypress.expose(). [2] - You can block new/remaining usage of the deprecated API with
allowCypressEnv: falsein config. [1][3]
Sources: Cypress changelog (15.10.0), migration guide, and cy.env() docs. [1][2][3]
Remove the spurious {timeout:…} argument from .should().
cy.get('#http-transport', {timeout: Cypress.env('largeTimeout')}).should('not.be.checked') is the correct pattern. Timeout passed to cy.get() is automatically inherited by .should(). The second {timeout: ...} argument is interpreted as Chai's expected value argument (not a Cypress options object), and since be.checked is a property assertion that takes no parameters, this argument is silently dropped. It's unnecessary and makes the intent unclear.
🐛 Proposed fix
-cy.get('#http-transport', {timeout: Cypress.env('largeTimeout')}).should('not.be.checked', {timeout: Cypress.env('largeTimeout')});
+cy.get('#http-transport', {timeout: Cypress.env('largeTimeout')}).should('not.be.checked');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@tests/cypress/e2e/publisher/003-run-time-configs/01-select-transport-types.cy.js`
at line 43, The test is passing a spurious `{timeout: ...}` object to
`.should()` on the cy.get('#http-transport', {timeout:
Cypress.env('largeTimeout')}) call; remove the second `{timeout: ...}` argument
so the assertion reads `.should('not.be.checked')` (the timeout stays on
`cy.get` and will be inherited), i.e., update the statement using the
'#http-transport' selector to drop the extra options object from `.should()`.
| }, () => { | ||
| // create a new api | ||
| cy.visit(`/publisher/apis/create/openapi`, { timeout: Cypress.config().largeTimeout }).wait(5000); | ||
| cy.visit(`/publisher/apis/create/openapi`, { timeout: Cypress.env('largeTimeout') }).wait(5000); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for largeTimeout definition in cypress config files
rg -n 'largeTimeout' --type js -C3Repository: wso2/apim-apps
Length of output: 50371
🏁 Script executed:
# Find and read cypress.config.js file
fd 'cypress.config.js' -type fRepository: wso2/apim-apps
Length of output: 228
🏁 Script executed:
# Check the specific test file lines mentioned in the review
fd '05-create-new-version-of-api-product.cy.js' --exec cat -n {} \; | sed -n '40,125p'Repository: wso2/apim-apps
Length of output: 5317
Consolidate timeout values using Cypress.env('largeTimeout').
Lines 109 and 111 use hardcoded { timeout: 30000 } while the rest of the file migrates to Cypress.env('largeTimeout') (100000ms from cypress.config.js). For consistency with the migration pattern, update these lines:
- cy.wait('@apiGet', { timeout: 30000 }).then(() => {
+ cy.wait('@apiGet', { timeout: Cypress.env('largeTimeout') }).then(() => {
// validate
- cy.get('#itest-api-name-version', { timeout: 30000 }).should('be.visible');
+ cy.get('#itest-api-name-version', { timeout: Cypress.env('largeTimeout') }).should('be.visible');Also, fix the typo on line 115: "verions" → "versions".
Also applies to: 54-54, 67-67, 84-84, 98-98, 120-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@tests/cypress/e2e/publisher/013-api-product/05-create-new-version-of-api-product.cy.js`
at line 43, Replace all hardcoded timeout objects `{ timeout: 30000 }` in the
Cypress test with the environment value `{ timeout: Cypress.env('largeTimeout')
}` so the file consistently uses the centralized timeout (update occurrences
around the clicks/visits at the same spots as the review: the instances near the
selectors used in this spec), and correct the typo "verions" to "versions"
(search for the string "verions" in this test and change it to "versions");
specifically update the cy.visit/cy.get calls that currently pass `{ timeout:
30000 }` to use `Cypress.env('largeTimeout')` and fix the misspelled label.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/cypress/support/commands.js (1)
1080-1141: Identical post-submit error-handling logic duplicated acrossdisableSelfSignUpInCarbonPortalandenableSelfSignUpInCarbonPortal.Lines 1092–1109 are byte-for-byte identical to lines 1124–1141. Extract to a shared helper.
♻️ Proposed refactor
Add a module-scoped helper before the command declarations:
+function handleSelfSignUpFormResult(tenant) { + cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(($body) => { + if ($body.text().includes('Error 403 - Forbidden')) { + cy.log(`Received 403 while saving self signup config for ${tenant}. Continuing.`); + return; + } + if ($body.find('.ui-dialog-buttonpane button').length > 0) { + cy.get('.ui-dialog-buttonpane button').first().click({ force: true }); + } else if ($body.find('[class="ui-button ui-corner-all ui-widget"]').length > 0) { + cy.get('[class="ui-button ui-corner-all ui-widget"]').first().click({ force: true }); + } + }); + cy.get('body').then(($body) => { + if ($body.find('[href="../admin/logout_action.jsp"]').length > 0) { + cy.carbonLogout(); + } else { + cy.visit('/carbon/admin/logout_action.jsp'); + } + }); +}Then in both commands, replace the duplicated block with:
cy.get('#idp-mgt-edit-local-form').submit(); - cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(($body) => { - ... - }); - cy.get('body').then(($body) => { - ... - }); + handleSelfSignUpFormResult(tenant); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/support/commands.js` around lines 1080 - 1141, The post-submit DOM/error handling block duplicated in Cypress.Commands 'disableSelfSignUpInCarbonPortal' and 'enableSelfSignUpInCarbonPortal' should be extracted to a shared helper (e.g., handlePostSubmitAndLogout or handleSaveDialog) placed module-scoped above the command definitions; implement the helper to accept the tenant name (for the 403 log) and encapsulate the two cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(...) dialog-handling branch plus the final cy.get('body').then(...) logout/visit logic, then replace the duplicated blocks in both functions by calling this helper with the tenant argument so both commands delegate to the single implementation.tests/cypress/e2e/devportal/000-general/03-self-signup.cy.js (1)
87-92: Optional: add a failure message and use.to.be.truefor debuggability.When neither condition is met, Cypress will report
"expected false to equal true"with no context about which URL or page content was observed.♻️ Proposed improvement
- expect(hasLegacyError || has404Error).to.equal(true); + expect(hasLegacyError || has404Error, + `Expected login-failed or 404 error on page, but got: "${pageText.slice(0, 200)}"` + ).to.be.true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/e2e/devportal/000-general/03-self-signup.cy.js` around lines 87 - 92, The current assertion inside the cy.get(...).should callback uses expect(hasLegacyError || has404Error).to.equal(true) without context; change it to expect(hasLegacyError || has404Error, `Unexpected page content: ${pageText}`).to.be.true so failures include the observed pageText for debugging; update the callback that defines pageText, hasLegacyError, and has404Error accordingly (the code block containing the cy.get(...).should and those variables).tests/cypress/support/pages/carbon/UsersManagementPage.js (1)
45-48: The CSS-escaped quote variants are redundant — consider trimming the selector.In JS,
`[onClick="deleteUser(\\'${userName}\\')"]`produces the CSS string[onClick="deleteUser(\'userName\')"]. In CSS,\'within a double-quoted attribute value is simply an escaped single quote ('), making it identical to[onClick="deleteUser('userName\')"]. The escaped variants match exactly the same DOM elements as the non-escaped variants; they don't match attributes with literal backslash characters. Keeping them misleads maintainers into thinking they handle a different case.♻️ Proposed simplification
static getDeleteButtonOfUser(userName){ return cy.get( - `[onClick="deleteUser('${userName}')"], [onClick="deleteUser(\\'${userName}\\')"], ` + - `[onclick="deleteUser('${userName}')"], [onclick="deleteUser(\\'${userName}\\')"]` + `[onClick="deleteUser('${userName}')"], [onclick="deleteUser('${userName}')"]` ) }The same change should be applied to the duplicate
deleteByOnClickSelectorstring intests/cypress/support/commands.js(lines 147–149).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/support/pages/carbon/UsersManagementPage.js` around lines 45 - 48, The selector string includes redundant CSS-escaped single-quote variants; remove the `\\'${userName}\\'` alternatives and keep only the unescaped attribute selectors so they match the same elements without misleading maintainers. Update the cy.get return in UsersManagementPage.js (the method that returns cy.get([...])) to remove `[onClick="deleteUser(\\'${userName}\\')"]` and `[onclick="deleteUser(\\'${userName}\\')"]`, and make the identical change to the duplicate deleteByOnClickSelector string in tests/cypress/support/commands.js so both use only the unescaped `[onClick="deleteUser('${userName}')"], [onclick="deleteUser('${userName}')"]` variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/cypress/support/commands.js`:
- Around line 845-861: The deleteApplication command currently visits
'/devportal/applications' then immediately checks the DOM, which races with the
AJAX that loads the applications list; update deleteApplication to mirror the
working deleteApp approach by adding a cy.intercept for the applications-list
network request (or another reliable API route that returns the list) before
cy.visit('/devportal/applications'), then call cy.wait for that intercept to
complete before querying the delete button selector
`#delete-${applicationName}-btn` and proceeding with the click/confirm flow;
ensure you reference the same intercept alias as used in deleteApp and keep the
existing delete/confirm logic intact.
---
Nitpick comments:
In `@tests/cypress/e2e/devportal/000-general/03-self-signup.cy.js`:
- Around line 87-92: The current assertion inside the cy.get(...).should
callback uses expect(hasLegacyError || has404Error).to.equal(true) without
context; change it to expect(hasLegacyError || has404Error, `Unexpected page
content: ${pageText}`).to.be.true so failures include the observed pageText for
debugging; update the callback that defines pageText, hasLegacyError, and
has404Error accordingly (the code block containing the cy.get(...).should and
those variables).
In `@tests/cypress/support/commands.js`:
- Around line 1080-1141: The post-submit DOM/error handling block duplicated in
Cypress.Commands 'disableSelfSignUpInCarbonPortal' and
'enableSelfSignUpInCarbonPortal' should be extracted to a shared helper (e.g.,
handlePostSubmitAndLogout or handleSaveDialog) placed module-scoped above the
command definitions; implement the helper to accept the tenant name (for the 403
log) and encapsulate the two cy.get('body', { timeout:
Cypress.env('largeTimeout') }).then(...) dialog-handling branch plus the final
cy.get('body').then(...) logout/visit logic, then replace the duplicated blocks
in both functions by calling this helper with the tenant argument so both
commands delegate to the single implementation.
In `@tests/cypress/support/pages/carbon/UsersManagementPage.js`:
- Around line 45-48: The selector string includes redundant CSS-escaped
single-quote variants; remove the `\\'${userName}\\'` alternatives and keep only
the unescaped attribute selectors so they match the same elements without
misleading maintainers. Update the cy.get return in UsersManagementPage.js (the
method that returns cy.get([...])) to remove
`[onClick="deleteUser(\\'${userName}\\')"]` and
`[onclick="deleteUser(\\'${userName}\\')"]`, and make the identical change to
the duplicate deleteByOnClickSelector string in
tests/cypress/support/commands.js so both use only the unescaped
`[onClick="deleteUser('${userName}')"], [onclick="deleteUser('${userName}')"]`
variants.
| Cypress.Commands.add('deleteApplication', (applicationName) => { | ||
| cy.get("#itest-link-to-applications", { timeout: Cypress.config().largeTimeout }).click(); | ||
| cy.get('table', { timeout: Cypress.config().largeTimeout }).get('tbody') | ||
| .get(`[data-testid="row-${applicationName}"]`) | ||
| .find('td').eq(5).get(`[id="delete-${applicationName}-btn"]`).click(); | ||
| cy.get("#itest-confirm-application-delete").click(); | ||
| const deleteButtonId = `#delete-${applicationName}-btn`; | ||
| cy.visit('/devportal/applications'); | ||
| cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(($body) => { | ||
| if ($body.find(deleteButtonId).length === 0) { | ||
| cy.log(`Application ${applicationName} is not found. Skipping delete.`); | ||
| return; | ||
| } | ||
|
|
||
| cy.get(deleteButtonId).first().click({ force: true }); | ||
| cy.get('body').then(($confirmBody) => { | ||
| if ($confirmBody.find('#itest-confirm-application-delete').length > 0) { | ||
| cy.get('#itest-confirm-application-delete').click({ force: true }); | ||
| } | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
deleteApplication is missing an applications-list load wait, making it unreliable.
cy.visit('/devportal/applications') loads the React SPA shell. Cypress waits for the page load event, but the applications list is populated via a subsequent AJAX call. By the time cy.get('body').then() fires, #delete-${applicationName}-btn is almost certainly not yet in the DOM. The command will log "not found" and skip the deletion on virtually every run, leaving stale applications in the test environment.
The existing deleteApp command (line 590) correctly uses cy.intercept + cy.wait for this reason.
🐛 Proposed fix
Cypress.Commands.add('deleteApplication', (applicationName) => {
const deleteButtonId = `#delete-${applicationName}-btn`;
+ cy.intercept('**/applications**').as('appListLoad');
cy.visit('/devportal/applications');
- cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(($body) => {
- if ($body.find(deleteButtonId).length === 0) {
- cy.log(`Application ${applicationName} is not found. Skipping delete.`);
- return;
- }
- cy.get(deleteButtonId).first().click({ force: true });
- cy.get('body').then(($confirmBody) => {
- if ($confirmBody.find('#itest-confirm-application-delete').length > 0) {
- cy.get('#itest-confirm-application-delete').click({ force: true });
- }
- });
- });
+ cy.wait('@appListLoad', { timeout: Cypress.env('largeTimeout') }).then(() => {
+ cy.get('body').then(($body) => {
+ if ($body.find(deleteButtonId).length === 0) {
+ cy.log(`Application ${applicationName} is not found. Skipping delete.`);
+ return;
+ }
+ cy.get(deleteButtonId).first().click({ force: true });
+ cy.get('body').then(($confirmBody) => {
+ if ($confirmBody.find('#itest-confirm-application-delete').length > 0) {
+ cy.get('#itest-confirm-application-delete').click({ force: true });
+ }
+ });
+ });
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Cypress.Commands.add('deleteApplication', (applicationName) => { | |
| cy.get("#itest-link-to-applications", { timeout: Cypress.config().largeTimeout }).click(); | |
| cy.get('table', { timeout: Cypress.config().largeTimeout }).get('tbody') | |
| .get(`[data-testid="row-${applicationName}"]`) | |
| .find('td').eq(5).get(`[id="delete-${applicationName}-btn"]`).click(); | |
| cy.get("#itest-confirm-application-delete").click(); | |
| const deleteButtonId = `#delete-${applicationName}-btn`; | |
| cy.visit('/devportal/applications'); | |
| cy.get('body', { timeout: Cypress.env('largeTimeout') }).then(($body) => { | |
| if ($body.find(deleteButtonId).length === 0) { | |
| cy.log(`Application ${applicationName} is not found. Skipping delete.`); | |
| return; | |
| } | |
| cy.get(deleteButtonId).first().click({ force: true }); | |
| cy.get('body').then(($confirmBody) => { | |
| if ($confirmBody.find('#itest-confirm-application-delete').length > 0) { | |
| cy.get('#itest-confirm-application-delete').click({ force: true }); | |
| } | |
| }); | |
| }); | |
| }); | |
| Cypress.Commands.add('deleteApplication', (applicationName) => { | |
| const deleteButtonId = `#delete-${applicationName}-btn`; | |
| cy.intercept('**/applications**').as('appListLoad'); | |
| cy.visit('/devportal/applications'); | |
| cy.wait('@appListLoad', { timeout: Cypress.env('largeTimeout') }).then(() => { | |
| cy.get('body').then(($body) => { | |
| if ($body.find(deleteButtonId).length === 0) { | |
| cy.log(`Application ${applicationName} is not found. Skipping delete.`); | |
| return; | |
| } | |
| cy.get(deleteButtonId).first().click({ force: true }); | |
| cy.get('body').then(($confirmBody) => { | |
| if ($confirmBody.find('#itest-confirm-application-delete').length > 0) { | |
| cy.get('#itest-confirm-application-delete').click({ force: true }); | |
| } | |
| }); | |
| }); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/cypress/support/commands.js` around lines 845 - 861, The
deleteApplication command currently visits '/devportal/applications' then
immediately checks the DOM, which races with the AJAX that loads the
applications list; update deleteApplication to mirror the working deleteApp
approach by adding a cy.intercept for the applications-list network request (or
another reliable API route that returns the list) before
cy.visit('/devportal/applications'), then call cy.wait for that intercept to
complete before querying the delete button selector
`#delete-${applicationName}-btn` and proceeding with the click/confirm flow;
ensure you reference the same intercept alias as used in deleteApp and keep the
existing delete/confirm logic intact.
d010e8b to
3c0e970
Compare
| "test:admin": "cypress run --e2e --spec \"cypress/e2e/admin/*.cy.js\"", | ||
| "test:publisher": "cypress run --e2e --spec \"cypress/e2e/publisher/013-api-product/*.cy.js\"" |
There was a problem hiding this comment.
| "test:admin": "cypress run --e2e --spec \"cypress/e2e/admin/*.cy.js\"", | |
| "test:publisher": "cypress run --e2e --spec \"cypress/e2e/publisher/013-api-product/*.cy.js\"" | |
| "test:admin": "cypress run --e2e --spec \"cypress/e2e/admin/**/*.cy.js\"", | |
| "test:publisher": "cypress run --e2e --spec \"cypress/e2e/publisher/**/*.cy.js\"" |
…e proper cleanup of tenant configurations
…e and improve operation policy retrieval
…prove element visibility before interaction
|
3 participants
--e2efor Cypress runs.aws-sdkwith@aws-sdk/client-s3and addedarchiverandnodemailerdependencies.Summary by CodeRabbit
Chores
Tests