Fix UI tests for new API key UIs and fix the other UI test failures#1274
Merged
Fix UI tests for new API key UIs and fix the other UI test failures#1274
Conversation
npamudika
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
|
Naduni Pamudika seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughRe-enables multiple previously skipped Cypress test suites, converts focused tests to normal execution, adds test helpers and resilient flows for app/API creation and key generation, improves conditional UI handling and cleanup, and hardens Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 8
🤖 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/integration/devportal/001-applications/03-generate-api-keys.spec.js`:
- Around line 47-64: The current check after cy.location falls through silently
if '#itest-info-bar-application-name' never appears, leaving appCreated false
and causing downstream errors; update the fallback branch (the inner
cy.get('body').then(...) block that inspects '#itest-info-bar-application-name')
to explicitly throw an Error when the element is not found instead of doing
nothing, so the test fails immediately and appCreated remains false — modify the
logic around cy.location(...) / cy.get('body') to assert presence or throw with
a clear message referencing '#itest-info-bar-application-name' and appCreated.
- Around line 29-32: The intercept for the application-attributes request is
registered after visiting the page in createAppForTest, so the request can be
missed; move the cy.intercept('**/application-attributes').as('attrGet') call to
occur before cy.visit('/devportal/applications/create?tenant=carbon.super') so
Cypress can capture the network request, and keep the subsequent
cy.wait('@attrGet', { timeout: 300000 }) to wait for that alias.
In
`@tests/cypress/integration/devportal/001-applications/04-exchange-key-managers.spec.js`:
- Line 19: The test visits the OAuth keys page before the intercept for
loadKeyManagers is registered and also uses the wrong fixture option name; move
the cy.intercept that creates the alias '@loadKeyManagers' to before the
cy.visit call in the spec so page-initiated requests hit the stubbed response,
and change any intercept option object property from "fixtures" to "fixture"
(e.g., update the throttling policy stub and any other cy.intercept(..., {
fixtures: '...' }) to cy.intercept(..., { fixture: '...' })) so Cypress serves
the static response correctly.
In
`@tests/cypress/integration/devportal/001-applications/05-test-application-sharing.spec.js`:
- Around line 180-187: The teardown must treat missing APIs as success: update
the afterEach cleanup so Utils.deleteAPI(apiId) and cy.deleteApi(apiName,
apiVersion) are idempotent by checking for existence and handling 404/not-found
without failing; for Utils.deleteAPI, catch the DELETE response and treat 404 as
success (log and return) and verify status before throwing, and for
cy.deleteApi, first query for the API card (or call the GET) and only attempt
deletion if present, otherwise continue without timing out or failing—adjust the
helper implementations (Utils.deleteAPI and cy.deleteApi) to return gracefully
on not-found and to surface actual errors only for real failures.
- Around line 118-132: The test currently logs and continues when neither
sharing UI exists, allowing a false positive; replace the silent fallback with
an assertion that at least one of the sharing controls exists by checking for
the selectors '#application-group-id' or 'input[aria-label="Share application
with the organization"]' before proceeding, then keep the existing branch logic
to interact with '#application-group-id' or the org-sharing switch; ensure the
assertion fails the spec if neither control is present so the test cannot pass
without configuring sharing.
- Around line 143-170: Replace the instantaneous if-check that uses
cy.get('body').then(($body) => { if ($body.text().includes(appName)) { ... }
else { ... } } ) with a proper Cypress assertion that retries: assert visibility
of the shared application using cy.contains(appName, { timeout:
Cypress.config().largeTimeout }).should('exist') (or .click() directly with that
timeout) and remove the else branch that logs and skips; then continue with the
same flow that clicks the app, extracts uuidApp from cy.location('pathname'),
performs subscriptions (buttons like '#left-menu-subscriptions',
'[data-testid="api-subscriptions-section"]', `#policy-subscribe-btn-${uuid}`)
and subsequent logout/login steps so the spec fails when user2 never sees the
app instead of silently passing.
In
`@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js`:
- Around line 98-103: In the after() cleanup hook, move the
cy.intercept('**/applications**').as('appGet') call so it runs before
cy.visit('/devportal/applications?tenant=carbon.super');; ensure the intercept
(alias 'appGet') is registered prior to the visit so the initial applications
request is captured and cy.wait('@appGet', { timeout: 300000 }) succeeds,
allowing the rest of the cleanup body (the cy.get('body') handler that deletes
created apps) to run reliably.
In
`@tests/cypress/integration/publisher/000-general/03-create-and-publish-graphql-api-with-all-information.spec.js`:
- Line 19: The test currently creates a fixed Carbon role named "FilmSubscriber"
in the "Create GraphQl API from file" suite and never removes it in the after()
hook, causing non-idempotent runs; update the setup to generate a unique role
name (e.g., append a timestamp or UUID) instead of the fixed "FilmSubscriber",
reuse that generated name in the local-scope step that references the role, and
ensure the after() teardown removes that same generated role so repeated or
parallel runs do not conflict with the created role.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5aa61826-b648-4b98-b8e9-2e76f6adc117
📒 Files selected for processing (9)
tests/cypress/integration/devportal/001-applications/02-generate-keys.spec.jstests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.jstests/cypress/integration/devportal/001-applications/04-exchange-key-managers.spec.jstests/cypress/integration/devportal/001-applications/05-test-application-sharing.spec.jstests/cypress/integration/devportal/002-subscriptions/01-subscribe-unsubscribe-to-app-from-app.spec.jstests/cypress/integration/devportal/002-subscriptions/03-change-subscription-tier-on-an-application.spec.jstests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.jstests/cypress/integration/publisher/000-general/03-create-and-publish-graphql-api-with-all-information.spec.jstests/cypress/integration/publisher/020-prototyped-apis/02-can-try-out-resources-with-key-when-security-enabled.spec.js
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js
Outdated
Show resolved
Hide resolved
tests/cypress/integration/devportal/001-applications/04-exchange-key-managers.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/001-applications/05-test-application-sharing.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/001-applications/05-test-application-sharing.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/001-applications/05-test-application-sharing.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js
Show resolved
Hide resolved
...gration/publisher/000-general/03-create-and-publish-graphql-api-with-all-information.spec.js
Show resolved
Hide resolved
PasanT9
previously approved these changes
Mar 13, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tests/cypress/support/commands.js (1)
554-568: Silent skip on missing app can mask cleanup failures.The guarded deletion logic is a reasonable defensive pattern. However, tests calling
cy.deleteApp()unconditionally inafter()hooks (e.g.,02-generate-keys.spec.jsas shown in context snippet 1) may leave orphan applications if the delete button selector doesn't match due to UI changes or timing issues, causing subsequent runs to fail on duplicate names.Consider either:
- Returning a status or throwing when deletion was expected but skipped (add an optional
expectExistsparameter)- Documenting this behavior so test authors know to track creation state themselves
🤖 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 554 - 568, The deleteApp command currently silently skips when the delete button selector is missing; update Cypress.Commands.add('deleteApp') to accept an optional boolean parameter (e.g., expectExists = false) so callers can require deletion: if expectExists is true and the deleteButtonSelector (`[id="delete-${appName}-btn"]`) is not found, fail the test by throwing or calling cy.wrap().then(() => { throw new Error(...) }) / cy.should('exist') with a clear message; otherwise preserve current silent behavior when expectExists is false and log the skip. Ensure the command signature and the internal body-checking logic around deleteButtonSelector and '#itest-confirm-application-delete' reflect this new parameter and that existing callers can opt in.tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js (1)
119-123: Consider replacing hardcoded waits with explicit assertions or intercepts.The
cy.wait(2000)andcy.wait(1000)calls introduce arbitrary delays and may cause flakiness. Prefer waiting for specific UI state changes or network responses.Example approach
openRuntimeConfigurations(); - cy.wait(2000); cy.get('#applicationLevel').click(); - cy.wait(1000); + cy.get('#api-security-api-key-checkbox').should('be.visible'); cy.get('#api-security-api-key-checkbox').check({ force: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js` around lines 119 - 123, Replace the hardcoded cy.wait(2000) and cy.wait(1000) with deterministic waits: intercept the network call triggered by clicking '#applicationLevel' or '#save-runtime-configurations' (using cy.intercept and alias) and wait on that alias, or assert UI readiness with selectors like cy.get('#applicationLevel').should('be.visible').click() and cy.get('#api-security-api-key-checkbox').should('exist').check({ force: true }) before clicking '#save-runtime-configurations'; remove the fixed waits and use cy.wait('@alias') or .should(...) assertions to ensure stable timing around the '#applicationLevel', '#api-security-api-key-checkbox', and '#save-runtime-configurations' interactions.tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js (1)
99-114: Inline deletion duplicates the centralizedcy.deleteApp()command.The after() hook reimplements the same logic that exists in
cy.deleteApp()(seetests/cypress/support/commands.jslines 554-568). Using the centralized command improves maintainability and ensures consistent behavior.Proposed simplification
after(() => { if (appCreated) { - cy.intercept('**/applications**').as('appGet'); - cy.visit('/devportal/applications?tenant=carbon.super'); - cy.wait('@appGet', { timeout: 300000 }).then(() => { - cy.get('body').then(($body) => { - if ($body.find(`#delete-${appName}-btn`).length > 0) { - cy.get(`#delete-${appName}-btn`).click({ force: true }); - cy.get('#itest-confirm-application-delete').click(); - } else { - cy.log(`Skipping app delete. Application not found: ${appName}`); - } - }); - }); + cy.deleteApp(appName); } else { cy.log(`Skipping app delete. Application creation did not complete: ${appName}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js` around lines 99 - 114, Replace the inline application-deletion logic in the after() hook with the centralized helper cy.deleteApp(...) to avoid duplication: remove the whole if (appCreated) { ... } else { ... } block and call the existing custom command cy.deleteApp(appName) (or cy.deleteApp() if that helper derives the name internally) so the test uses the single source of truth implemented in tests/cypress/support/commands.js (referenced function: cy.deleteApp; variables: appCreated, appName).
🤖 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/integration/devportal/001-applications/03-generate-api-keys.spec.js`:
- Around line 48-55: The assignment appCreated = true is executed synchronously
before the cy.wait('@createAppReq').then(...) verification completes; move the
appCreated = true assignment into the .then callback (after the expect checks)
so the flag is only set when the response exists and has an expected status;
update the block handling cy.wait('@createAppReq', { timeout: 120000 }).then(({
response }) => { ... }) to set appCreated = true after the two expect(...)
assertions.
In
`@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js`:
- Around line 31-34: The intercept for the application-attributes request is
registered after cy.visit in createAppForTest, so the network call during page
load can be missed; move the
cy.intercept('**/application-attributes').as('attrGet') call to before
cy.visit('/devportal/applications/create?tenant=carbon.super') in
createAppForTest so the request is captured, then call cy.wait('@attrGet', {
timeout: 300000 }) after the visit.
- Around line 56-66: The test silently continues when it cannot determine app
creation (pathname lacks "/overview" and the '#itest-info-bar-application-name'
selector isn't present), so update the body callback to fail fast: after
checking the pathname and then searching for '#itest-info-bar-application-name',
if neither branch sets appCreated to true, throw a clear error or call a Cypress
assertion to stop the test (reference the appCreated variable and the
'#itest-info-bar-application-name' selector in the code block inside the
cy.get('body').then(...)) so the test fails immediately with a helpful message
instead of proceeding.
---
Nitpick comments:
In
`@tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js`:
- Around line 119-123: Replace the hardcoded cy.wait(2000) and cy.wait(1000)
with deterministic waits: intercept the network call triggered by clicking
'#applicationLevel' or '#save-runtime-configurations' (using cy.intercept and
alias) and wait on that alias, or assert UI readiness with selectors like
cy.get('#applicationLevel').should('be.visible').click() and
cy.get('#api-security-api-key-checkbox').should('exist').check({ force: true })
before clicking '#save-runtime-configurations'; remove the fixed waits and use
cy.wait('@alias') or .should(...) assertions to ensure stable timing around the
'#applicationLevel', '#api-security-api-key-checkbox', and
'#save-runtime-configurations' interactions.
In
`@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js`:
- Around line 99-114: Replace the inline application-deletion logic in the
after() hook with the centralized helper cy.deleteApp(...) to avoid duplication:
remove the whole if (appCreated) { ... } else { ... } block and call the
existing custom command cy.deleteApp(appName) (or cy.deleteApp() if that helper
derives the name internally) so the test uses the single source of truth
implemented in tests/cypress/support/commands.js (referenced function:
cy.deleteApp; variables: appCreated, appName).
In `@tests/cypress/support/commands.js`:
- Around line 554-568: The deleteApp command currently silently skips when the
delete button selector is missing; update Cypress.Commands.add('deleteApp') to
accept an optional boolean parameter (e.g., expectExists = false) so callers can
require deletion: if expectExists is true and the deleteButtonSelector
(`[id="delete-${appName}-btn"]`) is not found, fail the test by throwing or
calling cy.wrap().then(() => { throw new Error(...) }) / cy.should('exist') with
a clear message; otherwise preserve current silent behavior when expectExists is
false and log the skip. Ensure the command signature and the internal
body-checking logic around deleteButtonSelector and
'#itest-confirm-application-delete' reflect this new parameter and that existing
callers can opt in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a31a8d94-d2fc-4d31-9125-e3fd2921b6c5
📒 Files selected for processing (3)
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.jstests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.jstests/cypress/support/commands.js
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js
Show resolved
Hide resolved
tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js
Show resolved
Hide resolved
PasanT9
previously approved these changes
Mar 13, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js (1)
98-119: Consider using the sharedcy.deleteApp()command for consistency.The after hook implements its own cleanup logic that duplicates the guarded delete pattern in
cy.deleteApp()fromcommands.js. Using the shared command would improve maintainability and ensure consistent behavior across test suites.Proposed refactor
after(() => { if (appCreated) { - cy.intercept('**/applications**').as('appGet'); - cy.visit('/devportal/applications?tenant=carbon.super'); - cy.wait('@appGet', { timeout: 300000 }).then(() => { - cy.get('body').then(($body) => { - if ($body.find(`#delete-${appName}-btn`).length > 0) { - cy.get(`#delete-${appName}-btn`).click({ force: true }); - cy.get('#itest-confirm-application-delete').click(); - } else { - cy.log(`Skipping app delete. Application not found: ${appName}`); - } - }); - }); + cy.deleteApp(appName); } else { cy.log(`Skipping app delete. Application creation did not complete: ${appName}`); } if (testApiId) { Utils.deleteAPI(testApiId); } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js` around lines 98 - 119, The after hook duplicates app deletion logic; replace the manual guarded DOM-based delete (the block referencing appCreated, appName, cy.intercept/cy.visit/cy.get(`#delete-${appName}-btn`), etc.) with the shared test command cy.deleteApp(appName) (or cy.deleteApp() as defined in commands.js) when appCreated is true to ensure consistent teardown; keep the existing Utils.deleteAPI(testApiId) call for API cleanup and preserve the appCreated guard so cy.deleteApp is only invoked if appCreated is truthy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.js`:
- Around line 98-119: The after hook duplicates app deletion logic; replace the
manual guarded DOM-based delete (the block referencing appCreated, appName,
cy.intercept/cy.visit/cy.get(`#delete-${appName}-btn`), etc.) with the shared
test command cy.deleteApp(appName) (or cy.deleteApp() as defined in commands.js)
when appCreated is true to ensure consistent teardown; keep the existing
Utils.deleteAPI(testApiId) call for API cleanup and preserve the appCreated
guard so cy.deleteApp is only invoked if appCreated is truthy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 457a99b8-58c2-496b-812d-9d6dc321ded5
📒 Files selected for processing (3)
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.jstests/cypress/integration/devportal/002-subscriptions/04-generate-keys-api-details-page.spec.jstests/cypress/support/commands.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/cypress/support/commands.js
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 68-95: The credentials selection currently builds rawFallbacks and
deduplicates into credentialsToTry which causes fallback accounts to be
attempted even when explicit username/password are provided; change the logic in
the commands.js flow that constructs rawFallbacks/credentialsToTry so that if
both username and password are truthy you only attempt that single explicit
credential (skip adding env/testUser/hardcoded fallbacks), otherwise build and
dedupe the fallbacks as before (references: rawFallbacks, credentialsToTry,
username, password, portal, testUsers). Also update the error handling where
login attempts are exhausted to emit distinct messages for "explicit credentials
failed" versus "all fallbacks exhausted" so test failures clearly show whether
the provided credential was rejected or no valid fallback existed (also apply
same change to the duplicate block at lines 119-125).
- Around line 624-626: The intercept matcher
cy.intercept('**/applications**').as('appGet') is too broad and can match
non-list calls; change it to a specific GET list intercept by using
cy.intercept({ method: 'GET', url: '/applications*' }) (or include the exact
query, e.g. '/applications?tenant=carbon.super') and keep the alias 'appGet',
then use cy.wait('@appGet') as before; update the same pattern in the other spec
(the one referencing 04-generate-keys-api-details-page.spec.js) so the wait only
resolves for the list endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c4e5e97-665d-4445-b9f1-1350ebbcef69
📒 Files selected for processing (2)
tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.jstests/cypress/support/commands.js
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/cypress/integration/devportal/001-applications/03-generate-api-keys.spec.js
PasanT9
approved these changes
Mar 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$subject
Fixes for wso2/api-manager#4642
Summary by CodeRabbit