Skip to content

Commit 75f1221

Browse files
nlespiaucqclaude
andauthored
fix(helpers): restore substring matching in waitInUrl (#5682)
#5451 wired resolveUrl() into waitInUrl alongside waitUrlEquals. For waitUrlEquals that is correct — a strict comparison needs the relative path resolved against the configured base url. For waitInUrl it silently turned a documented substring match into an origin-anchored one: const expectedUrl = resolveUrl(urlPart, this.options.url) // '/users' -> 'https://app.example.com/users' return currUrl.indexOf(expectedUrl) > -1 // scheme+host+port must now match exactly So I.waitInUrl('/users') breaks after a redirect to a different host, port or scheme, and a partial match such as I.waitInUrl('user=test') resolves to <base>/user=test and never matches /info?user=test. Compare against the raw urlPart again in Playwright, Puppeteer and WebDriver. waitUrlEquals is left untouched. The existing test asserted only inside catch, so it passed vacuously whenever the wait unexpectedly succeeded — which is how this shipped. It now fails on a missing timeout, plus a new case pinning the regression. docs/migration-4.md listed waitInUrl among the methods that resolve relative urls while its own example said the opposite; corrected. Co-authored-by: Claude <claude@anthropic.com>
1 parent 6ac157e commit 75f1221

5 files changed

Lines changed: 23 additions & 13 deletions

File tree

‎docs/migration-4.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ Test files written for 3.x keep working until you flip the flag.
614614
615615
### `wait*` Methods Resolve Relative URLs
616616
617-
`waitInUrl`, `waitUrlEquals`, and `waitCurrentPathEquals` now resolve a relative path against the helper's configured `url` before comparing. In 3.x a literal substring match against `window.location.href` would fail for relative paths.
617+
`waitUrlEquals` and `waitCurrentPathEquals` now resolve a relative path against the helper's configured `url` before comparing. In 3.x a literal comparison against `window.location.href` would fail for relative paths.
618618
619619
```js
620620
// helpers: { Playwright: { url: 'https://app.example.com' } }
@@ -623,6 +623,8 @@ I.waitUrlEquals('/dashboard') // matches https://app.example.com/dashboard
623623
I.waitInUrl('/users') // matches any URL containing /users
624624
```
625625
626+
`waitInUrl` is unchanged from 3.x — it stays a plain substring match against the current URL and never resolves its argument.
627+
626628
`waitUrlEquals` error messages now include the actual URL the page was on when the wait timed out — easier to diagnose `/dashboard` vs `/dashboard?session=expired`.
627629
628630
## 6. Adopt New Behaviors

‎lib/helper/Playwright.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3423,21 +3423,20 @@ class Playwright extends Helper {
34233423
*/
34243424
async waitInUrl(urlPart, sec = null) {
34253425
const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout
3426-
const expectedUrl = resolveUrl(urlPart, this.options.url)
34273426

34283427
return this.page
34293428
.waitForFunction(
34303429
urlPart => {
34313430
const currUrl = decodeURIComponent(decodeURIComponent(decodeURIComponent(window.location.href)))
34323431
return currUrl.indexOf(urlPart) > -1
34333432
},
3434-
expectedUrl,
3433+
urlPart,
34353434
{ timeout: waitTimeout },
34363435
)
34373436
.catch(async e => {
34383437
const currUrl = await this._getPageUrl()
34393438
if (/Timeout/i.test(e.message)) {
3440-
throw new Error(`expected url to include ${expectedUrl}, but found ${currUrl}`)
3439+
throw new Error(`expected url to include ${urlPart}, but found ${currUrl}`)
34413440
} else {
34423441
throw e
34433442
}

‎lib/helper/Puppeteer.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2501,7 +2501,6 @@ class Puppeteer extends Helper {
25012501
*/
25022502
async waitInUrl(urlPart, sec = null) {
25032503
const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout
2504-
const expectedUrl = resolveUrl(urlPart, this.options.url)
25052504

25062505
return this.page
25072506
.waitForFunction(
@@ -2510,12 +2509,12 @@ class Puppeteer extends Helper {
25102509
return currUrl.indexOf(urlPart) > -1
25112510
},
25122511
{ timeout: waitTimeout },
2513-
expectedUrl,
2512+
urlPart,
25142513
)
25152514
.catch(async e => {
25162515
const currUrl = await this._getPageUrl()
25172516
if (/Waiting failed:/i.test(e.message) || /failed: timeout/i.test(e.message)) {
2518-
throw new Error(`expected url to include ${expectedUrl}, but found ${currUrl}`)
2517+
throw new Error(`expected url to include ${urlPart}, but found ${currUrl}`)
25192518
} else {
25202519
throw e
25212520
}

‎lib/helper/WebDriver.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,23 +2521,22 @@ class WebDriver extends Helper {
25212521
async waitInUrl(urlPart, sec = null) {
25222522
const client = this.browser
25232523
const aSec = sec || this.options.waitForTimeoutInSeconds
2524-
const expectedUrl = resolveUrl(urlPart, this.options.url)
25252524
let currUrl = ''
25262525

25272526
return client
25282527
.waitUntil(
25292528
function () {
25302529
return this.getUrl().then(res => {
25312530
currUrl = decodeUrl(res)
2532-
return currUrl.indexOf(expectedUrl) > -1
2531+
return currUrl.indexOf(urlPart) > -1
25332532
})
25342533
},
25352534
{ timeout: aSec * 1000 },
25362535
)
25372536
.catch(e => {
25382537
e = wrapError(e)
25392538
if (e.message.indexOf('timeout')) {
2540-
throw new Error(`expected url to include ${expectedUrl}, but found ${currUrl}`)
2539+
throw new Error(`expected url to include ${urlPart}, but found ${currUrl}`)
25412540
}
25422541
throw e
25432542
})

‎test/helper/webapi.js‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,24 @@ export function tests() {
151151

152152
describe('#waitInUrl, #waitUrlEquals', () => {
153153
it('should wait part of the URL to match the expected', async () => {
154+
await I.amOnPage('/info')
155+
await I.waitInUrl('/info')
156+
await I.waitInUrl(`${siteUrl}/info`)
157+
158+
let err
154159
try {
155-
await I.amOnPage('/info')
156-
await I.waitInUrl('/info')
157160
await I.waitInUrl('/info2', 0.1)
158161
} catch (e) {
159-
assert.include(e.message, `expected url to include ${siteUrl}/info2, but found ${siteUrl}/info`)
162+
err = e
160163
}
164+
assert.isDefined(err, 'expected waitInUrl to time out')
165+
assert.include(err.message, `expected url to include /info2, but found ${siteUrl}/info`)
166+
})
167+
168+
it('should match a URL part that is not anchored at the base url', async () => {
169+
await I.amOnPage('/info?user=test')
170+
await I.waitInUrl('user=test')
171+
await I.waitInUrl('/info?user=test')
161172
})
162173

163174
it('should wait for the entire URL to match the expected', async () => {

0 commit comments

Comments
 (0)