Add unexpected toast message detection to Cypress tests#3147
Conversation
… visits / reloads
…unexpected-toasts # Conflicts: # tests/Frontend/e2e/AdminServersViewServerActions.cy.js # tests/Frontend/e2e/RoomsIndex.cy.js # tests/Frontend/e2e/RoomsViewDescription.cy.js # tests/Frontend/e2e/RoomsViewFiles.cy.js # tests/Frontend/e2e/RoomsViewFilesFileActions.cy.js # tests/Frontend/e2e/RoomsViewGeneral.cy.js # tests/Frontend/e2e/RoomsViewHistory.cy.js # tests/Frontend/e2e/RoomsViewMeetings.cy.js # tests/Frontend/e2e/RoomsViewMembers.cy.js # tests/Frontend/e2e/RoomsViewMembersBulkActions.cy.js # tests/Frontend/e2e/RoomsViewMembersMemberActions.cy.js # tests/Frontend/e2e/RoomsViewPersonalizedLinks.cy.js # tests/Frontend/e2e/RoomsViewPersonalizedLinksTokenActions.cy.js # tests/Frontend/e2e/RoomsViewRecordings.cy.js # tests/Frontend/e2e/RoomsViewRecordingsRecordingActions.cy.js # tests/Frontend/e2e/RoomsViewSettings.cy.js # tests/Frontend/e2e/RoomsViewStreaming.cy.js # tests/Frontend/e2e/RoomsViewStreamingConfigActions.cy.js
This reverts commit a19ed04.
…unexpected-toasts
…unexpected-toasts
…unexpected-toasts
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds test infrastructure to validate that Cypress e2e tests end without stray toast messages, and applies that validation to authorization and login test flows. A new ChangesE2E test toast message validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3147 +/- ##
=============================================
+ Coverage 96.80% 96.81% +0.01%
Complexity 1944 1944
=============================================
Files 466 466
Lines 13291 13291
Branches 2190 2190
=============================================
+ Hits 12866 12868 +2
+ Misses 425 423 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PILOS
 |
||||||||||||||||||||||||||||
| Project |
PILOS
|
| Branch Review |
cypress-check-for-unexpected-toasts
|
| Run status |
|
| Run duration | 07m 26s |
| Commit |
|
| Committer | Sabrina Wüst |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
632
|
| View all changes introduced in this branch ↗︎ | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/Frontend/support/commands/commandOverrides.js`:
- Around line 47-53: The Cypress.Commands.overwrite for "reload" currently only
forwards the second parameter named options to originalFn, breaking other call
signatures; update the overwrite in the Cypress.Commands.overwrite("reload",
(originalFn, options) => { ... }) implementation to forward all received
arguments to originalFn (e.g., capture rest args from the overwrite handler and
call originalFn(...args)) so any call patterns like cy.reload(forceReload,
options) are preserved; ensure the wrapper still performs the
wrap/checkFinalState/log steps before invoking originalFn with the full
arguments list.
- Around line 20-23: The wrapper around originalIt currently invokes testFn()
and cy.checkFinalState() without returning their results, losing any return
value from testFn and preventing Mocha from waiting for the Cypress chain;
update the it-wrapper (the function passed to originalIt) to preserve the test
context and return a combined chain/value by returning the result of calling
testFn() (or wrapping it in Promise.resolve if it may be non-promise) and then
chaining cy.checkFinalState() (or returning cy.checkFinalState() after testFn()
completes) so that originalIt receives the returned chain/value; locate the call
sites using originalIt, testFn, and cy.checkFinalState to implement this change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7695dd3f-a092-4288-8251-94729c0ad659
📒 Files selected for processing (7)
tests/Frontend/e2e/AdminIndex.cy.jstests/Frontend/e2e/AdminRolesNew.cy.jstests/Frontend/e2e/AdminRoomTypesNew.cy.jstests/Frontend/e2e/Login.cy.jstests/Frontend/support/commands/commandOverrides.jstests/Frontend/support/commands/generalCommands.jstests/Frontend/support/e2e.js
| return originalIt(title, testOptions, function () { | ||
| testFn(); | ||
| cy.checkFinalState(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether current tests rely on Mocha context (`this`) or done-callbacks in `it(...)`.
rg -nP --type=js -C2 '\bit\s*\([^,]+,\s*function\s*\([^)]*\)\s*\{' tests/Frontend/e2e
rg -nP --type=js -C2 '\bit\s*\([^,]+,\s*function\s*\(\s*done\s*\)' tests/Frontend/e2e
rg -nP --type=js -C2 '\bthis\.' tests/Frontend/e2eRepository: THM-Health/PILOS
Length of output: 50373
🏁 Script executed:
# First, examine the file in question
head -30 tests/Frontend/support/commands/commandOverrides.jsRepository: THM-Health/PILOS
Length of output: 1216
🏁 Script executed:
# Search for Mocha context usage (this.timeout, this.skip, this.retries) in test functions
rg -n 'this\.(timeout|skip|retries|slow)' tests/Frontend/e2e --max-count=20
# Search for done callbacks in test function parameters
rg -n 'it\([^,]+,\s*function\s*\(\s*done\s*\)' tests/Frontend/e2e --max-count=20
# Search for return statements in test functions (not cy.* chains)
rg -n 'return\s+(?!cy\.|this\.|.*\.)' tests/Frontend/e2e/*.js --max-count=20Repository: THM-Health/PILOS
Length of output: 302
🏁 Script executed:
# Simpler search for this. usage in test files
rg -n '\bthis\.' tests/Frontend/e2e --max-count=15
# Search for return statements in test functions (simple pattern)
rg -n '^\s+return\s+(?!cy)' tests/Frontend/e2e --max-count=15
# Search for promise-based patterns (.then, .catch)
rg -n 'testFn.*\.then|testFn.*\.catch' tests/Frontend/e2e --max-count=10Repository: THM-Health/PILOS
Length of output: 296
🏁 Script executed:
# Simple search for "this." in test files
rg 'this\.' tests/Frontend/e2e -c
# Count "return" statements in test files
rg '^\s+return' tests/Frontend/e2e -c
# Search for function(done) pattern specifically
rg 'function\s*\(\s*done\s*\)' tests/Frontend/e2e -c
# Look at a sample test to see the structure
head -100 tests/Frontend/e2e/UserProfile.cy.js | tail -50Repository: THM-Health/PILOS
Length of output: 1562
🏁 Script executed:
# Find which test files use "this."
rg 'this\.' tests/Frontend/e2e --files-with-matches
# Search for done callbacks more thoroughly
rg 'function\s*\(\s*done' tests/Frontend/e2eRepository: THM-Health/PILOS
Length of output: 42
🏁 Script executed:
# Search for cy.checkFinalState definition
rg 'checkFinalState' tests/Frontend/support -A 5
# Check if any test file returns a value from test function
rg 'it\([^,]+,.*function.*\{' tests/Frontend/e2e -A 10 | grep -E '(return|done)' | head -20
# Look for async/await in test functions
rg 'async\s+function' tests/Frontend/e2eRepository: THM-Health/PILOS
Length of output: 2095
🏁 Script executed:
# Check for any explicit return statements in the test function bodies
rg -n '^\s+return\s' tests/Frontend/e2e/*.js | head -20
# Check if any test files might return promises
rg -n 'return\s+(new\s+)?Promise' tests/Frontend/e2e | head -10
# Verify the full checkFinalState implementation
cat tests/Frontend/support/commands/generalCommands.js | grep -A 20 'checkFinalState'Repository: THM-Health/PILOS
Length of output: 1436
🏁 Script executed:
# Verify the exact code at lines 15-25 of commandOverrides.js
sed -n '15,25p' tests/Frontend/support/commands/commandOverrides.js
# Check if cy.visit override properly returns the chain
sed -n '32,50p' tests/Frontend/support/commands/commandOverrides.jsRepository: THM-Health/PILOS
Length of output: 1065
Preserve test context and return Cypress command chains in the it wrapper.
Line 21 calls testFn() directly without forwarding its return value, and line 22 calls cy.checkFinalState() but doesn't return its Cypress command chain. This causes two issues:
- If
testFn()returns a value (promise or otherwise), it's discarded, breaking tests that rely on return semantics. cy.checkFinalState()is async (usescy.url().then()internally), but since the wrapper doesn't return the chain, Mocha may not wait for it to complete, creating a race condition where the test finishes before final state validation runs.
Proposed fix
- return originalIt(title, testOptions, function () {
- testFn();
- cy.checkFinalState();
+ return originalIt(title, testOptions, function (...args) {
+ const result = testFn.apply(this, args);
+ return cy.checkFinalState().then(() => result);
});🤖 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 `@tests/Frontend/support/commands/commandOverrides.js` around lines 20 - 23,
The wrapper around originalIt currently invokes testFn() and
cy.checkFinalState() without returning their results, losing any return value
from testFn and preventing Mocha from waiting for the Cypress chain; update the
it-wrapper (the function passed to originalIt) to preserve the test context and
return a combined chain/value by returning the result of calling testFn() (or
wrapping it in Promise.resolve if it may be non-promise) and then chaining
cy.checkFinalState() (or returning cy.checkFinalState() after testFn()
completes) so that originalIt receives the returned chain/value; locate the call
sites using originalIt, testFn, and cy.checkFinalState to implement this change.
| /** | ||
| * Override for visit | ||
| * Checks final state before performing the visit, to make sure that unexpected errors that may have happened previously are caught. | ||
| */ | ||
| Cypress.Commands.overwrite("visit", (originalFn, url, options) => { | ||
| return cy | ||
| .wrap(null, { log: false }) | ||
| .then(() => cy.checkFinalState()) | ||
| .then(() => cy.log(`Visit ${url}`)) | ||
| .then(() => originalFn(url, options)); | ||
| }); | ||
|
|
||
| /** | ||
| * Override for reload | ||
| * Checks final state before performing the reload, to make sure that unexpected errors that may have happened previously are caught. | ||
| */ | ||
| Cypress.Commands.overwrite("reload", (originalFn, ...args) => { | ||
| return cy | ||
| .wrap(null, { log: false }) | ||
| .then(() => cy.checkFinalState()) | ||
| .then(() => cy.log("Reload")) | ||
| .then(() => originalFn(...args)); | ||
| }); |
There was a problem hiding this comment.
The cypress logs are changed quite a bit because of the overrides.
e.g.
Before the overrides:
visit $url
After the overrides:
log Visit $url
- then $url
This is the case because the originalFn is called inside a then and therefore not logged as e.g. visit anymore but as then (I am not sure why this is the case but didn't find any way to "fix" this).
The wrapping and calling originalFn is necessary because of the mix between calling a cypress custom command and calling originalFn which is not possible without the wrapping.
It would be possible to suppress the logs triggered by originalFn (the -then log) using log: false and also to partially "rebuild" a similar looking log entry using Cypress.log and through this get log entries that mostly look like the ones before.
Even though this would be possible I would suggests to not suppress the logs triggered by the originalFn because they can contain some additional information which could be potentially helpful for debugging. Because of that I would also not try to "rebuild" a similar looking log entry because it could be misleading and confusing why this log entry does not contain all the additional information contained in the original log entry.
Therefore I would suggest using the current log strategy.
|
There are still some explicit checks for missing toast messages at the end of tests, for example in These checks would also be covered by the added checks for unexpected toast messages and could therefore be removed. I currently kept them for now because in these tests the absence of a toast is part of the tested behavior, not just a generic cleanup/final-state condition. However, keeping only some explicit missing toast message checks at the end of tests may also be a bit confusing because other tests may rely only on |
1 participant
Fixes #
Type
Checklist
Changes
itcy.visit()andcy.reload()Summary by CodeRabbit