chore: fix-e2e

[ryanbas21]

JIRA Ticket

N/A

Description

Fixing test, getting rid of discouraged api, moving the order so that the page renders and auto waits for the element to appear

Summary by CodeRabbit

Release Notes

• Tests
  • Updated OIDC token endpoint tests to rely on default wait behavior for improved test consistency and reliability.
  • Reordered test assertions to align with updated flow sequence, ensuring accurate validation of authentication state transitions.

[changeset-bot]

⚠️ No Changeset found

Latest commit: a05e27a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[nx-cloud]

View your CI Pipeline Execution ↗ for commit a05e27a

Command	Status	Duration	Result
nx run-many -t build	✅ Succeeded	5s	View ↗
nx affected -t build typecheck lint test e2e-ci	✅ Succeeded	1m 23s	View ↗
nx-cloud record -- nx format:check	✅ Succeeded	1s	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-10-22 14:32:12 UTC

[coderabbitai]

Walkthrough

The pull request modifies end-to-end OIDC tests by removing explicit waitUntil: 'networkidle' options from three waitForURL calls and reordering assertions for code and state parameters to occur after subsequent test actions instead of immediately following navigation events.

Changes

Cohort / File(s)	Summary
OIDC E2E Test Assertion Reordering e2e/oidc-suites/src/token.spec.ts	Removed explicit waitUntil: 'networkidle' from three waitForURL calls; reordered URL-state assertions (code and state expectations) to occur after subsequent actions in the revoke-tokens flow rather than immediately post-navigation.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Test
    participant Browser
    
    rect rgb(220, 240, 220)
    note over Test,Browser: Before: Immediate assertion
    Test->>Browser: waitForURL (with networkidle)
    Browser-->>Test: Navigation complete
    Test->>Test: Assert code & state
    Test->>Browser: Perform next action
    end
    
    rect rgb(240, 220, 220)
    note over Test,Browser: After: Deferred assertion
    Test->>Browser: waitForURL (default behavior)
    Browser-->>Test: Navigation complete
    Test->>Browser: Perform next action
    Test->>Test: Assert code & state
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are localized to a single test file with straightforward modifications: removing configuration options and reordering test assertions. While the timing shift requires verification that the test logic remains correct, the changes follow a consistent pattern with minimal conceptual complexity.

Poem

🐰 A token test refined, no longer waits so long,
Assertions now deferred, the timing moves along,
The network idle fades, the default takes the crown,
Our OIDC flows now dance with lighter steps around town! 🎭

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title Check	❓ Inconclusive	The pull request title "chore: fix-e2e" is extremely vague and generic. While it does relate to the changes (the PR fixes e2e tests in the token.spec.ts file), the title conveys virtually no meaningful information about what was actually fixed or why. The specific changes involve removing a discouraged API option, reordering assertions, and adjusting wait behavior, none of which is captured in the title. A teammate scanning commit history would have minimal understanding of what this change addresses from the title alone.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The pull request description follows the required template structure with JIRA Ticket and Description sections present. The JIRA Ticket is appropriately marked as N/A, and the Description section contains substantive information about the changes: removing a discouraged API, reordering code to allow the page to render, and enabling automatic element waiting. While the description is brief and could benefit from more detail, it covers the essential aspects of what was changed and aligns with the actual modifications to the token.spec.ts file.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-e2e-test

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 259746b and a05e27a.

📒 Files selected for processing (1)

• e2e/oidc-suites/src/token.spec.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: pr

🔇 Additional comments (2)

e2e/oidc-suites/src/token.spec.ts (2)

80-84: Verify the reordering of URL parameter assertions.

The assertions for code and state parameters (lines 82-83) now occur after checking the access token (line 80), whereas in other similar tests (e.g., lines 31-32, 54-55, 119-120, 167-168) these assertions immediately follow the waitForURL call.

This reordering subtly changes the test semantics:

• New flow: Verify URL reached → verify access token exists → verify OAuth parameters
• Original flow: Verify URL reached → verify OAuth parameters → proceed with token operations

This inconsistency suggests either:

1. A race condition specific to PingAM's revoke flow where the access token appears before URL parameters are reliably verifiable
2. An incomplete refactor that should be applied to maintain consistency across all tests

Please confirm this reordering is intentional and whether the PingOne equivalent test (lines 166-170) should be updated similarly.

---

78-78: Verify the intentional removal of waitUntil: 'networkidle' at line 78.

The output confirms an inconsistency: line 78 removes waitUntil: 'networkidle' while all other waitForURL calls in this file retain it:

• PingAM tests: Line 30 and 53 have networkidle, but line 78 does not
• PingOne tests: Lines 118, 141, 166 all have networkidle

Without PR context explaining why this specific test required the change, it's unclear whether this is:

• An intentional fix for a timing issue in this test only
• An incomplete refactor that should apply to other tests
• An accidental omission

Removing waitUntil: 'networkidle' may cause timing/flakiness issues if not justified. Please confirm this is intentional.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new]

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/davinci-client@435

@forgerock/oidc-client

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/oidc-client@435

@forgerock/protect

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/protect@435

@forgerock/sdk-types

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-types@435

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-utilities@435

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/iframe-manager@435

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-logger@435

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-oidc@435

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/sdk-request-middleware@435

@forgerock/storage

pnpm add https://pkg.pr.new/ForgeRock/ping-javascript-sdk/@forgerock/storage@435

commit: a05e27a

[github-actions]

Deployed 77d335c to https://ForgeRock.github.io/ping-javascript-sdk/pr-435/77d335cb2ab9a2a0223d9b753b5cb42cf86f966f branch gh-pages in ForgeRock/ping-javascript-sdk

[github-actions]

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

🔻 @forgerock/journey-client - 0.0 KB (-82.0 KB, -100.0%)

📊 Minor Changes

📈 @forgerock/journey-client - 82.0 KB (+0.0 KB)

➖ No Changes

➖ @forgerock/device-client - 9.2 KB
➖ @forgerock/oidc-client - 23.0 KB
➖ @forgerock/protect - 150.1 KB
➖ @forgerock/sdk-utilities - 7.5 KB
➖ @forgerock/sdk-types - 8.0 KB
➖ @forgerock/storage - 1.4 KB
➖ @forgerock/sdk-logger - 1.6 KB
➖ @forgerock/iframe-manager - 2.4 KB
➖ @forgerock/sdk-request-middleware - 4.4 KB
➖ @forgerock/sdk-oidc - 2.5 KB
➖ @forgerock/davinci-client - 34.5 KB

---

13 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated

• Current Size: Total gzipped size of all files in the package's dist directory
• Baseline: Comparison against the latest build from the main branch
• Files included: All build outputs except source maps and TypeScript build cache
• Exclusions: .map, .tsbuildinfo, and .d.ts.map files

---

_{🔄 Updated automatically on each push to this PR}