Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/git-scm.spec.js: loosen book redirect assertions #1898

Conversation

ttaylorr
Copy link
Member

@ttaylorr ttaylorr commented Oct 6, 2024

In 8781699 (tests: verify that the book URL redirects work, 2024-09-26), there were a couple of assertions that were added to the Playwright test suite that look like the following:

await page.goto(`${url}book/`)
await expect(page).toHaveURL(`${url}book/en/v2`)

Or, in other words, expecting that going to the /book path relative to the site's base URL would result in a redirect to /book/env/v2.

But this test breaks when the hosted site is configured to redirect HTTP requests to HTTPS ones, like in this[1] example.

Loosen this assertion to just assert on the path component of the URL in an identical fashion to other similar assertions in this test. Note that a 'git grep toHaveURL' on the pre-image of this patch yields all but two assertions which only look at the path component. The two that don't are the ones which we modify here.

This should allow us to successfully run the Playwright tests in forks whose deployment configuration is similar to the above.

[1]: https://github.com/ttaylorr/git-scm.com/actions/runs/11185173194/artifacts/2017238933

/cc @dscho

In 8781699 (tests: verify that the book URL redirects work,
2024-09-26), there were a couple of assertions that were added to the
Playwright test suite that look like the following:

    await page.goto(`${url}book/`)
    await expect(page).toHaveURL(`${url}book/en/v2`)

Or, in other words, expecting that going to the `/book` path relative to
the site's base URL would result in a redirect to `/book/env/v2`.

But this test breaks when the hosted site is configured to redirect HTTP
requests to HTTPS ones, like in this[1] example.

Loosen this assertion to just assert on the path component of the URL in
an identical fashion to other similar assertions in this test. Note
that a 'git grep toHaveURL' on the pre-image of this patch yields all
but two assertions which only look at the path component. The two that
don't are the ones which we modify here.

This should allow us to successfully run the Playwright tests in forks
whose deployment configuration is similar to the above.

[1]: https://github.com/ttaylorr/git-scm.com/actions/runs/11185173194/artifacts/2017238933

Signed-off-by: Taylor Blau <me@ttaylorr.com>
@ttaylorr ttaylorr requested a review from dscho October 6, 2024 20:42
@ttaylorr
Copy link
Member Author

ttaylorr commented Oct 6, 2024

It works!

@ttaylorr ttaylorr marked this pull request as ready for review October 6, 2024 20:50
@ttaylorr
Copy link
Member Author

ttaylorr commented Oct 7, 2024

Closing in favor of #1899. Thanks, @dscho!

@ttaylorr ttaylorr closed this Oct 7, 2024
@ttaylorr ttaylorr deleted the ttaylorr/loosen-playwright-url-assertion branch October 7, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants