add better error handling, documentation, improve type safety, move selectors to selectors.ts

Author: hanna-meda

src/common/selectors.ts

@@ -293,4 +293,12 @@ export const selectors: Selectors = {
             }
         }
     }
-}
+}
+
+/**
+ * Global selectors for link validation and general page elements.
+ */
+export const linkValidationSelectors = {
+    allLinksInContent: '#wpbody-content a[href]',
+    tabLinksInContent: '#wpbody-content a[href*="page=wprocket#"]',
+} as const;

src/support/steps/broken-links.ts

@@ -3,19 +3,22 @@
  * This module contains Cucumber step definitions for validating links in WP Rocket settings UI.
  *
  * @requires {@link ../../common/custom-world}
- * @requires {@link @playwright/test}
  * @requires {@link @cucumber/cucumber}
+ * @requires {@link ../../common/selectors}
  */
 import { Then } from '@cucumber/cucumber';
 import { ICustomWorld } from '../../common/custom-world';
+import { linkValidationSelectors } from '../../common/selectors';
 
 /**
- * Executes the step to verify that WP Rocket settings links are not broken.
+ * Step definition that verifies WP Rocket settings links are not broken by collecting all links from settings tabs
+ * and checking their HTTP status codes. Fails on client errors (4xx) and logs warnings for server errors (5xx)
+ * to avoid flakiness from transient backend issues.
  *
  * @function
  * @async
  * @param {ICustomWorld} this - The Cucumber world context for the current scenario.
- * @return {Promise<void>} - A Promise that resolves when the check is completed.
+ * @return {Promise<void>} - A Promise that resolves when all links have been validated.
  */
 Then('WP Rocket settings links are not broken', async function (this: ICustomWorld) {
     const hrefs = new Set<string>();
@@ -24,7 +27,7 @@ Then('WP Rocket settings links are not broken', async function (this: ICustomWor
     await this.utils.visitPage('wp-admin/options-general.php?page=wprocket');
 
     const collectLinks = async (): Promise<void> => {
-        const links = await this.page.$$eval('#wpbody-content a[href]', (elements) =>
+        const links = await this.page.$$eval(linkValidationSelectors.allLinksInContent, (elements) =>
             elements
                 .map((element) => element.getAttribute('href'))
                 .filter((href): href is string => Boolean(href))
@@ -39,16 +42,19 @@ Then('WP Rocket settings links are not broken', async function (this: ICustomWor
     await collectLinks();
 
     // Collect tab links from within the WP Rocket settings content.
-    const tabHrefs = await this.page.$$eval('#wpbody-content a[href*="page=wprocket#"]', (links) =>
+    // Links are collected from each tab separately because different tabs may display different content
+    // and therefore different links. This ensures comprehensive link coverage across all settings sections.
+    const tabHrefs = await this.page.$$eval(linkValidationSelectors.tabLinksInContent, (links) =>
         links
             .map((link) => link.getAttribute('href'))
-            .filter(Boolean)
+            .filter((href): href is string => Boolean(href))
     );
 
     const tabUrls = Array.from(new Set(tabHrefs)).map((href) => {
-        return new URL(href as string, this.page.url()).toString();
+        return new URL(href, this.page.url()).toString();
     });
 
+    // Visit each tab and collect its links to ensure all links across all settings sections are validated
     for (const tabUrl of tabUrls) {
         await this.page.goto(tabUrl);
         await this.page.waitForLoadState('load');
@@ -97,30 +103,33 @@ Then('WP Rocket settings links are not broken', async function (this: ICustomWor
         try {
             const response = await this.page.request.get(url, { maxRedirects: 5, timeout: 30000 });
 
-            // Only fail on 404 - this indicates a truly broken link
-            if (response.status() === 404) {
-                throw new Error(`Link is broken: ${url} returned 404`);
+            const status = response.status();
+            
+            // Treat all client errors (4xx) as hard failures, as they usually indicate broken or unauthorized links.
+            // This includes 401/403, which can reveal authentication/authorization problems, not just 404 "not found".
+            if (status >= 400 && status < 500) {
+                throw new Error(`Client error: ${url} returned status ${status}`);
             }
 
-            // For other non-2xx/3xx status codes (like 5xx), log but don't fail
-            if (response.status() >= 400) {
+            // For server errors (5xx), log but don't fail to avoid flakiness from transient backend issues.
+            if (status >= 500) {
                 // eslint-disable-next-line no-console
                 console.warn(
-                    `Warning: ${url} returned status ${response.status()}, but continuing test (only 404s fail)`
+                    `Warning: ${url} returned server error status ${status}, but continuing test to avoid flakiness.`
                 );
             }
         } catch (error) {
             const message = error instanceof Error ? error.message : String(error);
 
-            // If this is our explicit 404 error, re-throw it to fail the test
-            if (message.includes('returned 404')) {
+            // If this is a client error (4xx), re-throw it to fail the test.
+            if (message.startsWith('Client error:')) {
                 throw error;
             }
 
-            // For network errors (timeouts, connection issues, etc.), log but don't fail
+            // For network errors (timeouts, connection issues, etc.), log but don't fail.
             // eslint-disable-next-line no-console
             console.warn(
-                `Warning: Network error while requesting ${url}: ${message}. Continuing test (only 404s fail).`
+                `Warning: Network or non-client error while requesting ${url}: ${message}. Continuing test.`
             );
         }
     }