[Devportal] Improve e2e tests - #2900
Conversation
📝 WalkthroughWalkthroughThe PR expands Developer Portal Cypress coverage with shared resource-seeding commands, mock OAuth token support, public browsing tests, sidebar checks, application and key-management flows, and active local-login scenarios. ChangesDeveloper Portal E2E coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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: 8
🧹 Nitpick comments (4)
portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured administrator username.
cy.login()readsCypress.env('ADMIN_USER'), but both tests assumeadmin. Use that environment value for the profile assertion and invalid-password submission so the suite remains portable.Proposed fix
+ const adminUser = Cypress.env('ADMIN_USER'); cy.login(); - cy.get('.profile-link').should('be.visible').and('contain', 'admin'); + cy.get('.profile-link').should('be.visible').and('contain', adminUser); ... - cy.get('`#username`').type('admin'); + cy.get('`#username`').type(Cypress.env('ADMIN_USER'));Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js` at line 35, Update both administrator username assertions and submissions in the tests around cy.login() to use Cypress.env('ADMIN_USER') instead of the hardcoded 'admin' value, including the profile-link assertion and invalid-password case.portals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.js (2)
74-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSearch-narrowing tests duplicate the MCP listing spec's logic.
These two search-narrowing tests are structurally identical (same shape, different fixture names/selectors) to the ones in
003-mcp-servers/001-mcp-listing.cy.js(lines 65-82). Consider extracting a shared Cypress command for "type a query, assert matching card visible / non-matching card absent" to avoid drift as more listing types are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.js` around lines 74 - 94, The search assertions in “narrows the listing with a search query” and “does not surface MCP servers via API search” duplicate the MCP listing spec’s query-and-card verification. Extract a reusable Cypress command for entering a search query, confirming the URL contains the query, and asserting matching or excluded cards, then update both API and MCP listing tests to use it.
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated listing-search assertions into a shared Cypress command. Both specs implement the identical "type a query into
#query, assert the URL containsquery=, assert the matching card is visible and the non-matching one is absent" pattern against symmetric listing pages (APIs vs MCPs).
portals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.js#L74-94: replace the two search tests with a shared command, e.g.cy.assertListingSearch(matchName, excludedName).portals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/001-mcp-listing.cy.js#L65-82: replace the two search tests with the same shared command.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.js` at line 1, Extract the duplicated listing-search assertion flow from the API and MCP listing specs into a shared Cypress command named assertListingSearch(matchName, excludedName). Have the command type the query in `#query`, verify the URL contains query=, assert the matching card is visible, and assert the excluded card is absent; replace both search tests in each spec with calls to this command.portals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/001-mcp-listing.cy.js (1)
65-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSearch-narrowing tests duplicate the API listing spec's logic.
These search-narrowing tests are structurally identical (same shape, different fixture names/selectors) to
002-apis/001-api-listing.cy.js(lines 74-94). Consider extracting a shared Cypress command for this pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/001-mcp-listing.cy.js` around lines 65 - 82, Extract the repeated search-and-filter assertion pattern from the MCP tests and the corresponding API listing spec into a shared Cypress command. Update the MCP cases around “narrows the listing with a search query” and “does not surface REST APIs via MCP search” plus the API listing tests to use that command, preserving their existing fixtures and expected visibility results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@portals/developer-portal/it/ui/cypress/e2e/002-apis/002-rest-api-details.cy.js`:
- Around line 109-128: Add an assertion in the OpenAPI specification test around
the elements-api apiDescriptionDocument check to verify the expected OAuth2
scopes, including read:items and write:items. Keep the existing /items path
assertion and update the nearby comment only if needed to accurately describe
the added coverage.
In
`@portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js`:
- Around line 47-49: Update the beforeEach hook’s uncaught:exception handler to
suppress only the specific known error that requires it, rather than returning
false for every application exception; allow all other exceptions to propagate
and fail the Cypress test.
- Around line 40-44: Update the application cleanup in the after() hook to
assert that deleting the reusable “IT Detail App” returns the expected 200
status. Do not silently suppress other response statuses; only permit a
documented idempotent 404 if required, while surfacing unexpected cleanup
failures.
In `@portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js`:
- Around line 24-26: Remove the unconditional uncaught:exception handler from
the beforeEach setup in the file-based login test, or narrow it to a documented,
specific expected exception signature while allowing all other browser
exceptions to fail the test.
- Around line 54-57: Update the error-message assertion in the file-based login
Cypress test to require the mandated generic authentication-failure message,
“Invalid or expired credentials.”, matching the `loginError` rendered by the
login template. Keep the visibility and `/login` URL assertions unchanged.
In `@portals/developer-portal/it/ui/cypress/support/commands/applications.js`:
- Around line 40-41: Update createApplication() and deleteApplication() to
locate applications by exact text in the .app-card-name element rather than
substring matching the entire .app-card. Reuse the exact-matched card for
visibility waits and handle retrieval, and in deleteApplication() operate on
that card’s closest delete control.
In `@portals/developer-portal/it/ui/mock-token-server.js`:
- Around line 66-68: Update the request-body collection in the mock endpoint
around the req data/end listeners to enforce a small maximum size; stop
accumulating and abort the request once the limit is exceeded, then return a
generic HTTP 413 response. Preserve normal parsing and handling for bodies
within the cap.
- Around line 85-106: The mock token server startup must reject listen failures
instead of resolving stale metadata, and startup should explicitly error when a
server is already running. Update the startup function’s error and
existing-server handling accordingly, then change stopMockTokenServer to return
a promise resolved by server.close’s callback, clearing server and meta only
after shutdown completes so callers can await termination.
---
Nitpick comments:
In `@portals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.js`:
- Around line 74-94: The search assertions in “narrows the listing with a search
query” and “does not surface MCP servers via API search” duplicate the MCP
listing spec’s query-and-card verification. Extract a reusable Cypress command
for entering a search query, confirming the URL contains the query, and
asserting matching or excluded cards, then update both API and MCP listing tests
to use it.
- Line 1: Extract the duplicated listing-search assertion flow from the API and
MCP listing specs into a shared Cypress command named
assertListingSearch(matchName, excludedName). Have the command type the query in
`#query`, verify the URL contains query=, assert the matching card is visible, and
assert the excluded card is absent; replace both search tests in each spec with
calls to this command.
In
`@portals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/001-mcp-listing.cy.js`:
- Around line 65-82: Extract the repeated search-and-filter assertion pattern
from the MCP tests and the corresponding API listing spec into a shared Cypress
command. Update the MCP cases around “narrows the listing with a search query”
and “does not surface REST APIs via MCP search” plus the API listing tests to
use that command, preserving their existing fixtures and expected visibility
results.
In `@portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js`:
- Line 35: Update both administrator username assertions and submissions in the
tests around cy.login() to use Cypress.env('ADMIN_USER') instead of the
hardcoded 'admin' value, including the profile-link assertion and
invalid-password case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af4fa2a1-6973-4c51-bf7b-68e5b3ef18ef
📒 Files selected for processing (17)
portals/developer-portal/it/ui/cypress.config.jsportals/developer-portal/it/ui/cypress/e2e/001-basic/001-portal-access.cy.jsportals/developer-portal/it/ui/cypress/e2e/001-basic/002-sidebar.cy.jsportals/developer-portal/it/ui/cypress/e2e/002-apis/001-api-listing.cy.jsportals/developer-portal/it/ui/cypress/e2e/002-apis/002-rest-api-details.cy.jsportals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/001-mcp-listing.cy.jsportals/developer-portal/it/ui/cypress/e2e/003-mcp-servers/tryout.cy.jsportals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.jsportals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.jsportals/developer-portal/it/ui/cypress/support/commands/applications.jsportals/developer-portal/it/ui/cypress/support/commands/auth.jsportals/developer-portal/it/ui/cypress/support/commands/index.jsportals/developer-portal/it/ui/cypress/support/commands/portal.jsportals/developer-portal/it/ui/cypress/support/commands/seed.jsportals/developer-portal/it/ui/mock-token-server.jsportals/developer-portal/src/pages/login-page/partials/signin-card.hbsportals/developer-portal/src/scripts/signin.js
| // Scopes section renders. NOTE: the DB-backed overview passes scopes: [] | ||
| // unconditionally (apiContentController.loadAPIContent), so this section | ||
| // shows the empty state here; the actual OAuth2 scopes surface in the | ||
| // specification view (Stoplight Elements), asserted separately below. | ||
| cy.contains('.aov-section-title', 'Scopes').should('exist'); | ||
| }); | ||
|
|
||
| it('opens the documentation and renders the OpenAPI specification', () => { | ||
| cy.visitPortal(`/api/${apiHandle}`); | ||
| cy.get('a.dp-btn[href$="/docs/specification"]').click(); | ||
|
|
||
| cy.contains('.page-title', 'Documentation').should('be.visible'); | ||
| // The OpenAPI spec is embedded in the Stoplight Elements web component, | ||
| // carrying the API document (server-modified but still containing paths). | ||
| cy.get('.adoc-file-badge--spec').should('contain', 'openapi'); | ||
| cy.get('elements-api').should('exist'); | ||
| cy.get('elements-api') | ||
| .invoke('attr', 'apiDescriptionDocument') | ||
| .should('include', '/items'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Comment promises a scope assertion that doesn't exist.
The comment at Lines 109-112 states OAuth2 scopes are "asserted separately below" in the specification test, but the specification test (Lines 116-128) only checks that apiDescriptionDocument includes /items — no assertion checks for read:items/write:items scope content anywhere in this file. Either add the missing scope assertion or correct the comment so it doesn't overstate coverage.
✅ Proposed fix: add the missing scope assertion
cy.get('elements-api')
.invoke('attr', 'apiDescriptionDocument')
- .should('include', '/items');
+ .should('include', '/items')
+ .and('include', 'read:items');📝 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.
| // Scopes section renders. NOTE: the DB-backed overview passes scopes: [] | |
| // unconditionally (apiContentController.loadAPIContent), so this section | |
| // shows the empty state here; the actual OAuth2 scopes surface in the | |
| // specification view (Stoplight Elements), asserted separately below. | |
| cy.contains('.aov-section-title', 'Scopes').should('exist'); | |
| }); | |
| it('opens the documentation and renders the OpenAPI specification', () => { | |
| cy.visitPortal(`/api/${apiHandle}`); | |
| cy.get('a.dp-btn[href$="/docs/specification"]').click(); | |
| cy.contains('.page-title', 'Documentation').should('be.visible'); | |
| // The OpenAPI spec is embedded in the Stoplight Elements web component, | |
| // carrying the API document (server-modified but still containing paths). | |
| cy.get('.adoc-file-badge--spec').should('contain', 'openapi'); | |
| cy.get('elements-api').should('exist'); | |
| cy.get('elements-api') | |
| .invoke('attr', 'apiDescriptionDocument') | |
| .should('include', '/items'); | |
| }); | |
| // Scopes section renders. NOTE: the DB-backed overview passes scopes: [] | |
| // unconditionally (apiContentController.loadAPIContent), so this section | |
| // shows the empty state here; the actual OAuth2 scopes surface in the | |
| // specification view (Stoplight Elements), asserted separately below. | |
| cy.contains('.aov-section-title', 'Scopes').should('exist'); | |
| }); | |
| it('opens the documentation and renders the OpenAPI specification', () => { | |
| cy.visitPortal(`/api/${apiHandle}`); | |
| cy.get('a.dp-btn[href$="/docs/specification"]').click(); | |
| cy.contains('.page-title', 'Documentation').should('be.visible'); | |
| // The OpenAPI spec is embedded in the Stoplight Elements web component, | |
| // carrying the API document (server-modified but still containing paths). | |
| cy.get('.adoc-file-badge--spec').should('contain', 'openapi'); | |
| cy.get('elements-api').should('exist'); | |
| cy.get('elements-api') | |
| .invoke('attr', 'apiDescriptionDocument') | |
| .should('include', '/items') | |
| .and('include', 'read:items'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@portals/developer-portal/it/ui/cypress/e2e/002-apis/002-rest-api-details.cy.js`
around lines 109 - 128, Add an assertion in the OpenAPI specification test
around the elements-api apiDescriptionDocument check to verify the expected
OAuth2 scopes, including read:items and write:items. Keep the existing /items
path assertion and update the nearby comment only if needed to accurately
describe the added coverage.
| cy.request({ | ||
| method: 'DELETE', | ||
| url: `/api/v0.9/applications/${detailHandle}`, | ||
| failOnStatusCode: false, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js"
if [ -f "$file" ]; then
echo "=== file exists and line count ==="
wc -l "$file"
echo
echo "=== outline ==="
ast-grep outline "$file" --view compact || true
echo
echo "=== relevant file contents ==="
sed -n '1,190p' "$file" | cat -n
else
echo "missing $file"
echo "candidate files:"
fd -a 'application-flows.cy.js$' . || true
fiRepository: wso2/api-platform
Length of output: 9622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== search app fixtures / fixed application names ==="
rg -n "application|app_?handle|handle|APP_NAME|application name|DELETE|cy\.request|failOnStatusCode" portals/developer-portal/it/ui/cypress package.json cypress 2>/dev/null || true
echo
echo "=== locate app-related e2e files ==="
fd -a 'application.*\.cy\.js$|api.*\.js$|support.*\.js$' portals/developer-portal/it/ui/cypress 2>/dev/null | sed 's#^\./##' | sortRepository: wso2/api-platform
Length of output: 10344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== support command files ==="
fd -a 'e2e.js|api.js|index.js|applications.js|seed.js' portals/developer-portal/it/ui/cypress/support | sort
echo
echo "=== command implementations ==="
for f in \
portals/developer-portal/it/ui/cypress/support/e2e.js \
portals/developer-portal/it/ui/cypress/support/commands/api.js \
portals/developer-portal/it/ui/cypress/support/commands/index.js \
portals/developer-portal/it/ui/cypress/support/commands/applications.js \
portals/developer-portal/it/ui/cypress/support/commands/seed.js
do
if [ -f "$f" ]; then
echo "===== $f ($ (wc -l < "$f") lines) ====="
sed -n '1,260p' "$f" | cat -n
fi
done
echo
echo "=== application API path references ==="
rg -n "/api/v0\.9/applications|createApplication|deleteApplication|DELETE.*application|application.*DELETE|failOnStatusCode" portals/developer-portal/it/ui/cypressRepository: wso2/api-platform
Length of output: 22783
🏁 Script executed:
#!/bin/bash
set -u
echo "=== backend DELETE application references ==="
rg -n "DELETE|deleteApplication|applications/:|/applications" portals/developer-portal/src portals/developer-portal/api portals/developer-portal/test 2>/dev/null | sed -n '1,220p'
echo
echo "=== application controller/service candidates ==="
fd -i 'application.*\.(js|ts|java)$|.*app.*(controller|service|route).*\.(js|ts|java)$' portals/developer-portal/src portals/developer-portal/api 2>/dev/null | sort | sed -n '1,120p'Repository: wso2/api-platform
Length of output: 20904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== applicationsHandler.js ==="
sed -n '1,140p' portals/developer-portal/src/routes/api/handlers/applicationsHandler.js | cat -n
echo
echo "=== devportalController.js delete sections ==="
sed -n '150,260p' portals/developer-portal/src/controllers/devportalController.js | cat -n
echo
echo "=== applicationDao.js relevant methods ==="
sed -n '60,115p' portals/developer-portal/src/dao/applicationDao.js | cat -n
sed -n '145,230p' portals/developer-portal/src/dao/applicationDao.js | cat -nRepository: wso2/api-platform
Length of output: 13815
Assert fixture cleanup succeeds.
after() creates a reusable app named IT Detail App, and the API delete only returns 200 for success. The failOnStatusCode: false cleanup silently accepts failures, so reruns can retain a stale fixture or collide on the fixed app name. Assert the intended success status, or explicitly allow only documented idempotent 404 cleanup statuses and log unexpected failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js`
around lines 40 - 44, Update the application cleanup in the after() hook to
assert that deleting the reusable “IT Detail App” returns the expected 200
status. Do not silently suppress other response statuses; only permit a
documented idempotent 404 if required, while surfacing unexpected cleanup
failures.
| beforeEach(() => { | ||
| cy.on('uncaught:exception', () => false); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files matching target:"
git ls-files | rg 'portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js$' || true
echo
echo "Target excerpt with line numbers:"
if [ -f portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js ]; then
sed -n '1,120p' portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js | nl -ba | sed -n '1,120p'
else
echo "target file missing"
fi
echo
echo "Search for uncaught exception handlers in Cypress tests:"
rg -n "uncaught:exception|cy\.on\(|on\('uncaught:exception'" portals/developer-portal/it/ui/cypress || trueRepository: wso2/api-platform
Length of output: 339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target excerpt with line numbers:"
awk 'NR<=160 {printf "%6d %s\n", NR, $0}' portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js || true
echo
echo "Search for uncaught exception handlers in Cypress tests:"
rg -n "uncaught:exception|cy\.on\(|on\('uncaught:exception'" portals/developer-portal/it/ui/cypress || true
echo
echo "Other exception / error handlers nearby:"
rg -n "catch|try|fail|uncaught|exception|console" portals/developer-portal/it/ui/cypress/e2e/applications/app* 2>/dev/null || trueRepository: wso2/api-platform
Length of output: 9845
🌐 Web query:
Cypress uncaught:exception callback returns false suppress test failure documentation
💡 Result:
In Cypress, when an uncaught exception occurs in your application, the test will automatically fail [1][2]. To suppress this failure, you can listen to the uncaught:exception event and return false from the callback function [1][3]. By returning false, you instruct Cypress to ignore the error and allow the test to continue execution [1][4]. Implementation Details: Global vs. Local Scope: - To apply the handler to all tests, use Cypress.on('uncaught:exception',...) [5][6]. This is commonly placed in your support file (e.g., cypress/support/e2e.js) [5]. - To apply the handler only to a specific test or block, use cy.on('uncaught:exception',...) [5][6]. Example of ignoring all uncaught exceptions: Cypress.on('uncaught:exception', (err, runnable) => { // Returning false prevents Cypress from failing the test return false }) Best Practice: It is generally recommended to only suppress specific, expected errors rather than ignoring all exceptions, which could mask genuine bugs [1][4][5]. You can do this by checking the error message: Cypress.on('uncaught:exception', (err, runnable) => { // Only ignore if the error message contains the expected text if (err.message.includes('expected error message')) { return false } // Otherwise, return true (or do nothing) to let the test fail as expected }) Important Notes: - Cross-origin: If the uncaught exception occurs within a cy.origin command, the handler must be defined inside that cy.origin block to take effect [1][3]. - Test Failures vs. App Errors: The uncaught:exception event only applies to errors thrown by your application code [6]. It does not suppress failures caused by test code (e.g., assertion failures or command timeouts), which are handled by the fail event [6].
Citations:
- 1: https://docs.cypress.io/api/cypress-api/catalog-of-events
- 2: https://docs.cypress.io/app/references/error-messages
- 3: https://github.com/cypress-io/cypress-documentation/blob/main/docs/api/cypress-api/catalog-of-events.mdx
- 4: https://github.com/cypress-io/cypress-example-recipes/tree/master/examples/fundamentals__errors
- 5: https://qa-essentials.com/cypress-error-handling-the-best-practice-with-examples/
- 6: https://stackoverflow.com/questions/68001586/cypress-on-uncaught-exception-is-not-working
Do not suppress every uncaught application exception.
cy.on('uncaught:exception', () => false) lets any frontend crash continue and can mask real bugs. If this suppression is required, narrow it to a specific known-error case instead of returning false universally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js`
around lines 47 - 49, Update the beforeEach hook’s uncaught:exception handler to
suppress only the specific known error that requires it, rather than returning
false for every application exception; allow all other exceptions to propagate
and fail the Cypress test.
| beforeEach(() => { | ||
| cy.on('uncaught:exception', () => false); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate target file and related support commands:"
fd -a 'file-based-login\.cy\.js|auth\.js' portals/developer-portal/it/ui/cypress 2>/dev/null | sed 's#^\./##'
echo
echo "Target file:"
if [ -f portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js ]; then
wc -l portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js
cat -n portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js
fi
echo
echo "Support command:"
AUTH_CMD="portals/developer-portal/it/ui/cypress/support/commands/auth.js"
if [ -f "$AUTH_CMD" ]; then
wc -l "$AUTH_CMD"
cat -n "$AUTH_CMD"
fi
echo
echo "Search for comparable uncaught exception handlers in Cypress tests/support:"
rg -n "uncaught:exception|cy\.on\('uncaught|return false" portals/developer-portal/it/ui/cypress || trueRepository: wso2/api-platform
Length of output: 8322
Do not suppress every uncaught browser exception.
Returning false unconditionally allows client-side regressions to pass as green tests. Remove this handler, or restrict it to one documented, expected exception signature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js`
around lines 24 - 26, Remove the unconditional uncaught:exception handler from
the beforeEach setup in the file-based login test, or narrow it to a documented,
specific expected exception signature while allowing all other browser
exceptions to fail the test.
| cy.get('.ln-error-banner') | ||
| .should('be.visible') | ||
| .and('contain', 'Invalid username or password'); | ||
| cy.url().should('include', '/login'); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Assert the required generic authentication-failure message.
This pins the UI to Invalid username or password, although the login template renders loginError directly. Align it with the mandated generic message so the test does not codify a divergent authentication contract.
Proposed fix
- .and('contain', 'Invalid username or password');
+ .and('contain', 'Invalid or expired credentials.');As per coding guidelines, “All authentication failures must return identical HTTP 401 Unauthorized responses with { "error": "unauthorized", "message": "Invalid or expired credentials." }.”
📝 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.
| cy.get('.ln-error-banner') | |
| .should('be.visible') | |
| .and('contain', 'Invalid username or password'); | |
| cy.url().should('include', '/login'); | |
| cy.get('.ln-error-banner') | |
| .should('be.visible') | |
| .and('contain', 'Invalid or expired credentials.'); | |
| cy.url().should('include', '/login'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@portals/developer-portal/it/ui/cypress/e2e/auth/file-based-login.cy.js`
around lines 54 - 57, Update the error-message assertion in the file-based login
Cypress test to require the mandated generic authentication-failure message,
“Invalid or expired credentials.”, matching the `loginError` rendered by the
login template. Keep the visibility and `/login` URL assertions unchanged.
Source: Coding guidelines
| cy.contains('.app-card', name, { timeout: 15000 }).should('be.visible'); | ||
| return cy.contains('.app-card', name).invoke('attr', 'data-id'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 "createApplication\(|deleteApplication\(" \
portals/developer-portal/it/ui/cypressRepository: wso2/api-platform
Length of output: 3264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== applications support file =="
cat -n portals/developer-portal/it/ui/cypress/support/commands/applications.js
echo
echo "== application-flow specs =="
cat -n portals/developer-portal/it/ui/cypress/e2e/applications/application-flows.cy.js
echo
echo "== search for create/delete apps and app names =="
rg -n -C3 "createApplication|deleteApplication|\.app-card-name|\.app-card\\s|data-id" \
portals/developer-portal/it/ui/cypressRepository: wso2/api-platform
Length of output: 20539
🌐 Web query:
Cypress cy.contains substring search documentation
💡 Result:
In Cypress, the cy.contains command performs a substring match by default when searching for text [1][2]. Key details regarding this behavior include: - Substring Matching: If you pass a string to cy.contains, it searches for any element that contains that string as a substring [1]. For example, cy.contains('Hello') will match an element containing the text 'Hello World' [1]. - Case Sensitivity: By default, the search is case-sensitive [3]. You can modify this behavior by passing an options object: cy.contains('Hello', { matchCase: false }) [3]. - Regular Expressions: For more complex or flexible matching (such as partial matches in specific patterns), you can pass a Regular Expression [4][3]. For example, cy.contains(/Find.* hero/) can be used to match elements containing 'Find' and 'hero' with any characters in between [5]. - Selecting Multiple Elements: cy.contains yields only the first matching element [4]. If you need to find all elements containing a specific substring, you can use the jQuery:contains selector with cy.get or.filter, such as cy.get('li:contains("text")') [6][4]. Always remember that cy.contains is a query command that retries until the element is found or the timeout expires [2][3].
Citations:
- 1: https://docs.cypress.io/api/commands/contains
- 2: https://dev.to/sebastianclavijo/the-second-most-abused-and-misused-cypress-command-ever-cycontains-126j
- 3: https://docs.w3cub.com/cypress/api/commands/contains.html
- 4: https://glebbahmutov.com/cypress-examples/recipes/find-by-class-or-text.html
- 5: https://stackoverflow.com/questions/74108021/cypress-find-part-of-a-string-with-cy-contains
- 6: https://docs.cypress.io/api/commands/filter
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
src = Path("portals/developer-portal/it/ui/cypress/support/commands/applications.js").read_text()
cases = [
("foo", "foo"),
("foo", "foo-backup"),
("foo-backup", "foo"),
("CRM", "CRM Admin"),
("CRM Admin", "CRM"),
]
for needle, el_text in cases:
print(f"{needle!r} includes {el_text!r}: {needle in el_text}")
print(
"delete filter:"
"",
src[src.index("const card = $body.find('.app-card').filter") :
src.index("if (!card.length)"]
).strip()
)
print(
"create/return lines:"
"",
"\n".join(src.splitlines()[39:42])
)
PYRepository: wso2/api-platform
Length of output: 327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
src = Path("portals/developer-portal/it/ui/cypress/support/commands/applications.js").read_text()
needle, el_text = "foo", "foo-backup"
print("createApplication uses cy.contains(.app-card, name): substring selects foo-backup for foo:", needle in el_text)
needle, el_text = "foo-backup", "foo"
print("deleteApplication filter uses textContent.includes(name): selects foo-backup for foo:", needle in el_text)
print("deleteApplication filter includes matches non-consecutive text, e.g. 'fo obar' in 'foo bar':", "fo obar" in "foo bar")
lines = src.splitlines()
print("createApplication lines 40-42:")
for n in range(40, 43):
print(f"{n}: {lines[n-1]}")
print("deleteApplication filter lines 51-56:")
for n in range(51, 57):
print(f"{n}: {lines[n-1]}")
PYRepository: wso2/api-platform
Length of output: 979
Match application labels exactly before yielding or deleting.
createApplication() and deleteApplication() accept any app card whose text contains the requested name, so foo can match foo-backup and delete createApplication("foo", ...) can return foo-backup’s handle. Use .app-card-name exact-text matching for the wait/return/delete paths and operate on the matched card’s closest delete control.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@portals/developer-portal/it/ui/cypress/support/commands/applications.js`
around lines 40 - 41, Update createApplication() and deleteApplication() to
locate applications by exact text in the .app-card-name element rather than
substring matching the entire .app-card. Reuse the exact-matched card for
visibility waits and handle retrieval, and in deleteApplication() operate on
that card’s closest delete control.
| let body = ''; | ||
| req.on('data', (chunk) => { body += chunk; }); | ||
| req.on('end', () => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the mock endpoint request body.
This listener accumulates an unbounded body while exposed to the test Docker network. Enforce a small cap, abort collection after exceeding it, and return a generic 413.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@portals/developer-portal/it/ui/mock-token-server.js` around lines 66 - 68,
Update the request-body collection in the mock endpoint around the req data/end
listeners to enforce a small maximum size; stop accumulating and abort the
request once the limit is exceeded, then return a generic HTTP 413 response.
Preserve normal parsing and handling for bodies within the cap.
Source: Coding guidelines
| server.on('error', (err) => { | ||
| console.error('[mock-token-server] failed to start:', err.message); | ||
| resolve(meta); // The round-trip test fails loudly on its own if unreachable. | ||
| }); | ||
| server.listen(MOCK_TOKEN_PORT, '0.0.0.0', () => { | ||
| meta = { | ||
| endpoint: `http://${containerIpv4()}:${MOCK_TOKEN_PORT}/token`, | ||
| secret: MOCK_TOKEN_SECRET, | ||
| accessToken: MOCK_ACCESS_TOKEN, | ||
| }; | ||
| resolve(meta); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function stopMockTokenServer() { | ||
| if (server) { | ||
| server.close(); | ||
| server = null; | ||
| meta = null; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 "cy\.task\(['\"](startMockTokenServer|stopMockTokenServer)" \
portals/developer-portal/it/ui/cypressRepository: wso2/api-platform
Length of output: 1880
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate mock-token-server =="
fd -a 'mock-token-server\.js$' .
echo "== file outline =="
ast-grep outline portals/developer-portal/it/ui/mock-token-server.js || true
echo "== relevant file lines =="
cat -n portals/developer-portal/it/ui/mock-token-server.js | sed -n '1,180p'
echo "== task registrations/usages =="
rg -n "startMockTokenServer|stopMockTokenServer|token-server" portals/developer-portal/it -SRepository: wso2/api-platform
Length of output: 7040
🌐 Web query:
Node.js http.Server close callback error event listen error callback behavior
💡 Result:
In Node.js, the 'close' event, server.listen callbacks, and 'error' event handling serve distinct roles in an http.Server lifecycle: 1. server.close([callback]): The 'close' event is emitted when the server stops accepting new connections and all existing connections are closed [1][2]. The optional callback passed to server.close is executed only after the server has successfully closed [2]. If the server was already closed, the callback will be invoked with an ERR_SERVER_NOT_RUNNING error [1]. Importantly, server.close does not forcibly terminate active connections; if connections remain open (e.g., due to keep-alive), the 'close' event and callback may be delayed indefinitely [2]. To ensure a graceful shutdown, you can use server.closeAllConnections or server.closeIdleConnections (available in Node.js v18.2.0+) to terminate active or idle connections [2]. 2. server.listen and Error Handling: The callback provided to server.listen is executed when the server has successfully begun listening [3]. It is not a standard error-first callback; if an error occurs during the listen process (such as EADDRINUSE), the callback will not receive the error [3]. Instead, you must explicitly attach an 'error' event listener to the server instance to handle startup errors (e.g., server.once('error', (err) => {... })) [3]. 3. 'clientError' event: When a client connection emits an 'error' (e.g., due to a malformed request), it is forwarded to the server's 'clientError' event [1][4]. The listener is responsible for closing or destroying the socket [1][4]. A common pattern is to check if the socket is still writable before attempting to send a 400 Bad Request response [5][4]. In summary, do not rely on the server.listen callback for error reporting; use an 'error' event listener. For server shutdown, be aware that server.close waits for all connections to finish unless you manually close them [2].
Citations:
- 1: https://nodejs.org/api/http.html
- 2: https://stackoverflow.com/questions/28053659/why-is-server-close-callback-never-invoked
- 3: https://stackoverflow.com/questions/53079611/handling-node-http-serverlisten-error-with-callback
- 4: https://github.com/nodejs/node/blob/master/doc/api/http.md
- 5: https://nodejs.org/docs/latest-v18.x/api/http.html
Fail startup explicitly and await server shutdown.
On listen failure, error resolves null but leaves server set, so a subsequent task start sees a stale server and returns null again. Reject startup failures and treat an already-running server as an explicit error. Also don’t clear server/meta before server.close() finishes; return a promise that resolves from the close callback so after() waits for shutdown before the spec exits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@portals/developer-portal/it/ui/mock-token-server.js` around lines 85 - 106,
The mock token server startup must reject listen failures instead of resolving
stale metadata, and startup should explicitly error when a server is already
running. Update the startup function’s error and existing-server handling
accordingly, then change stopMockTokenServer to return a promise resolved by
server.close’s callback, clearing server and meta only after shutdown completes
so callers can await termination.
1 participant
Purpose
Improving the e2e tests
Approach
Added e2e tests for: