exclude /renew and /upgrade URLs from broken link validation#344
exclude /renew and /upgrade URLs from broken link validation#344hanna-meda wants to merge 1 commit intodevelopfrom
Conversation
nicomollet
left a comment
nicomollet
left a comment
There was a problem hiding this comment.
✅ I believe we are good, the links list doesn't show URLs that would create orders on WPR website.
There was a problem hiding this comment.
Pull request overview
This PR updates the WP Rocket settings broken-link E2E check to avoid validating /renew/ and /upgrade/ URLs, preventing the test suite from triggering transactional side effects on wp-rocket.me.
Changes:
- Add skip-pattern support to the shared
validateLinks()helper. - Exclude
/renew/and/upgrade/links from the WP Rocket settings link collection/validation flow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| utils/helpers.ts | Extends validateLinks() with optional skipPatterns filtering. |
| src/support/steps/broken-links.ts | Filters out /renew/ and /upgrade/ URLs before running the broken-link validation step. |
| /** | ||
| * Validates HTTP/HTTPS URLs and collects broken links, handling client/server errors appropriately. | ||
| * Fails on internal 4xx errors, allows external 401/403 (gated content), warns on 5xx and network errors. | ||
| * | ||
| * @async | ||
| * @param {Page} page - The Playwright page object | ||
| * @param {Set<string>} urls - Set of URLs to validate | ||
| * @param {string} currentHost - Current host to distinguish internal from external URLs | ||
| * @return {Promise<string[]>} - Array of broken link strings in format "STATUS: url" | ||
| */ | ||
| export const validateLinks = async (page: Page, urls: Set<string>, currentHost: string): Promise<string[]> => { | ||
| export const validateLinks = async ( | ||
| page: Page, | ||
| urls: Set<string>, | ||
| currentHost: string, | ||
| skipPatterns: RegExp[] = [] | ||
| ): Promise<string[]> => { |
There was a problem hiding this comment.
validateLinks now accepts skipPatterns but the JSDoc wasn’t updated to document this new parameter. Please add an @param entry for skipPatterns (and describe expected matching semantics) so callers know how to use it.
| let normalizedUrls = normalizeUrls(allHrefs, basePageUrl); | ||
|
|
||
| // Exclude URLs that may trigger transactional side effects | ||
| const skipPatterns = [/\/renew\//i, /\/upgrade\//i]; | ||
| normalizedUrls = new Set( | ||
| Array.from(normalizedUrls).filter(url => !skipPatterns.some(pattern => pattern.test(url))) | ||
| ); |
There was a problem hiding this comment.
The URL skipping logic is duplicated: you filter normalizedUrls with skipPatterns here, but validateLinks also has built-in skipPatterns support (currently not used because it’s called without the 4th arg). Consider keeping the skip logic in one place by either (a) removing this manual filtering and passing skipPatterns into validateLinks, or (b) dropping the skipPatterns parameter from validateLinks if you want the step to own filtering.
Description
This PR skips /renew/ and /upgrade/ URLs in WP Rocket settings E2E broken link tests to prevent triggering unintended transactional actions.
Fixes #343
Type of change
Detailed scenario
What was tested
Added a temporarily log of the URLs being validated and ran command
npm run test:brokenlinksConfirmed /renew/ and /upgrade/ are excluded.
How to test
Run command
npm run test:brokenlinksAffected Features & Quality Assurance Scope
Technical description
Documentation
The step definition now filters out URLs containing /renew/ or /upgrade/ before validating their HTTP status.
New dependencies
None.
Risks
Minimal risk: Only skips specific URLs, does not affect other test logic.
Mandatory Checklist
Code validation
Code style
Unticked items justification
N/A
Additional Checks