Skip to content

test: fixes e2e tests for notification channels - #3407

Merged
stalniy merged 1 commit into
mainfrom
test/fix-alerts
Jul 10, 2026
Merged

test: fixes e2e tests for notification channels#3407
stalniy merged 1 commit into
mainfrom
test/fix-alerts

Conversation

@stalniy

@stalniy stalniy commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Why

this tests are broken now

What

Summary by CodeRabbit

  • Tests
    • Improved notification channel coverage by confirming a success message appears after creation.
    • Added dismissal handling for the success alert.
    • Ensured verification checks the final pagination page so newly created channels are found reliably.

@stalniy
stalniy marked this pull request as ready for review July 10, 2026 09:17
@stalniy
stalniy enabled auto-merge July 10, 2026 09:17
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Notification channel UI verification

Layer / File(s) Summary
Creation feedback and final-page verification
apps/deploy-web/tests/ui/pages/NotificationChannelsPage.ts, apps/deploy-web/tests/ui/managed-wallet-notification-channels.spec.ts
The page object can navigate to the last pagination page, while the test acknowledges the creation alert and uses the helper before verifying the channel.

Estimated code review effort: 2 (Simple) | ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/fix-alerts

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.85%. Comparing base (23f29fa) to head (ab94546).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3407      +/-   ##
==========================================
- Coverage   71.51%   70.85%   -0.67%     
==========================================
  Files        1110     1060      -50     
  Lines       28131    26820    -1311     
  Branches     7013     6717     -296     
==========================================
- Hits        20119    19002    -1117     
+ Misses       7027     6853     -174     
+ Partials      985      965      -20     
Flag Coverage Δ *Carryforward flag
api 85.60% <ø> (ø) Carriedforward from 23f29fa
deploy-web 60.42% <ø> (ø)
notifications 91.44% <ø> (ø) Carriedforward from 23f29fa
provider-console 81.38% <ø> (ø) Carriedforward from 23f29fa
provider-inventory ?
provider-proxy 86.42% <ø> (ø) Carriedforward from 23f29fa

*This pull request uses carry forward flags. Click here to find out more.
see 50 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@apps/deploy-web/tests/ui/pages/NotificationChannelsPage.ts`:
- Around line 32-35: Replace the immediate isVisible() check in the pagination
handling logic with an explicit Playwright wait for the pagination navigation to
become visible, using a short timeout and catching timeout errors to return when
pagination is genuinely absent. Ensure the fallback preserves the current
early-return behavior while allowing slow-rendering pagination to be detected.
🪄 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

Run ID: 381a7abd-07fb-4041-b353-99760c1246b7

📥 Commits

Reviewing files that changed from the base of the PR and between 23f29fa and ab94546.

📒 Files selected for processing (2)
  • apps/deploy-web/tests/ui/managed-wallet-notification-channels.spec.ts
  • apps/deploy-web/tests/ui/pages/NotificationChannelsPage.ts

Comment thread apps/deploy-web/tests/ui/pages/NotificationChannelsPage.ts

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Auto-approved: all changes are related to tests.

@stalniy
stalniy added this pull request to the merge queue Jul 10, 2026
Merged via the queue into main with commit 7faff13 Jul 10, 2026
57 checks passed
@stalniy
stalniy deleted the test/fix-alerts branch July 10, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant