Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tests/Frontend/e2e/AdminIndex.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe("Admin index", function () {
// Check if the welcome page is shown
cy.url().should("not.include", "/admin");
cy.get("h1").should("be.visible").and("include.text", "home.title");
cy.checkToastMessage("app.flash.unauthorized");
});

it("check admin index with all permissions", function () {
Expand Down
1 change: 1 addition & 0 deletions tests/Frontend/e2e/AdminRolesNew.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe("Admin roles new", function () {
// Check if the welcome page is shown
cy.url().should("not.include", "/admin/room_types");
cy.get("h1").should("be.visible").and("include.text", "home.title");
cy.checkToastMessage("app.flash.unauthorized");
});

it("add new role", function () {
Expand Down
1 change: 1 addition & 0 deletions tests/Frontend/e2e/AdminRoomTypesNew.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe("Admin room types new", function () {
// Check if the welcome page is shown
cy.url().should("not.include", "/admin/room_types");
cy.get("h1").should("be.visible").and("include.text", "home.title");
cy.checkToastMessage("app.flash.unauthorized");
});

it("add new room type", function () {
Expand Down
2 changes: 2 additions & 0 deletions tests/Frontend/e2e/Login.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,7 @@ describe("Login", function () {
cy.visit("/external_login");

cy.url().should("include", "/rooms").and("not.include", "/login");
cy.checkToastMessage("auth.flash.login");
});

it("hide shibboleth login if disabled", function () {
Expand Down Expand Up @@ -823,6 +824,7 @@ describe("Login", function () {
cy.visit("/external_login");

cy.url().should("include", "/rooms").and("not.include", "/login");
cy.checkToastMessage("auth.flash.login");
});

it("hide oidc login if disabled", function () {
Expand Down
53 changes: 53 additions & 0 deletions tests/Frontend/support/commands/commandOverrides.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Override for window.it used for all tests, to check the final state after each test, to make sure that unexpected errors
// or states that may have happened during the test are caught.

const originalIt = window.it;

window.it = function (title, optionsOrFunction, fn) {
// Determine test function and options based on the arguments passed to it
const testFn =
typeof optionsOrFunction === "function" ? optionsOrFunction : fn;
const testOptions =
typeof optionsOrFunction === "object" ? { ...optionsOrFunction } : {};

if (typeof testFn !== "function") {
// Call the original it function with the provided arguments
// This is necessary to make it.skip work correctly
return originalIt(title, optionsOrFunction, fn);
}

// Call the original it function with the modified test function and options
return originalIt(title, testOptions, function () {
testFn();
cy.checkFinalState();
});
Comment on lines +20 to +23
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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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/e2e

Repository: THM-Health/PILOS

Length of output: 50373


🏁 Script executed:

# First, examine the file in question
head -30 tests/Frontend/support/commands/commandOverrides.js

Repository: 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=20

Repository: 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=10

Repository: 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 -50

Repository: 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/e2e

Repository: 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/e2e

Repository: 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.js

Repository: 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:

  1. If testFn() returns a value (promise or otherwise), it's discarded, breaking tests that rely on return semantics.
  2. cy.checkFinalState() is async (uses cy.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.

};

// Add the original it.only and it.skip methods to the window object
// originalIt.only and originalIt.skip will use the overridden window.it function
window.it.only = originalIt.only;
window.it.skip = originalIt.skip;

/**
* 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));
});
Comment on lines +31 to +53
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

22 changes: 22 additions & 0 deletions tests/Frontend/support/commands/generalCommands.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,25 @@
/**
* Check that the final state of the test is correct, e.g. that there are no unexpected error toasts.
* This should be called at the end of every test to make sure that unexpected errors aren't missed in the tests.
* @memberof cy
* @method checkFinalState
* @returns void
*/
Cypress.Commands.add("checkFinalState", () => {
cy.url({ log: false }).then((currentUrl) => {
const currentOrigin = new URL(currentUrl).origin;
const baseOrigin = new URL(Cypress.config().baseUrl).origin;

if (currentOrigin === baseOrigin) {
cy.log("Check final state");
// Check that all toasts that maybe have been created during the test have been removed
// This makes sure that unexpected error toasts that are not handled in the test will cause the test to fail,
// which makes sure that we don't miss any unexpected errors in the tests
cy.get(".p-toast-message").should("not.exist");
}
});
});

/**
* Check that a user who visits this page without being logged in is redirected to the login page
* @memberof cy
Expand Down
1 change: 1 addition & 0 deletions tests/Frontend/support/e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// https://on.cypress.io/configuration
// ***********************************************************

import "./commands/commandOverrides.js";
import "./commands/generalCommands.js";
import "./commands/roomCommands.js";
import "./commands/interceptCommands.js";
Expand Down
Loading