test(docs-infra): fix double-slash in URL of aio_monitoring test
#25641
Conversation
|
You can preview 51483b7 at https://pr25641-51483b7.ngbuilds.io/. |
As part of the tests run in the CircleCI `aio_monitoring` job, we need to retrieve `sitemap.xml` from the site under test. Previously, the URL used to retrieve that contained a double-slash (`//`). At some point, Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail. This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes.
51483b7 to
267c60e
Compare
|
You can preview 267c60e at https://pr25641-267c60e.ngbuilds.io/. |
|
Where do the notifications that this monitoring task are failing go? |
| @@ -49,6 +49,7 @@ export function loadLocalSitemapUrls() { | |||
| } | |||
|
|
|||
| export async function loadRemoteSitemapUrls(host: string) { | |||
| host = host.replace(/\/$/, ''); | |||
There was a problem hiding this comment.
we could just use path.join(host, 'generated/sitemap.xml') on the following line instead?
There was a problem hiding this comment.
| @@ -44,7 +47,7 @@ export class SitePage { | |||
| .then(regs => Promise.all(regs.map(reg => reg.unregister()))) | |||
| .then(cb); | |||
|
|
|||
| await browser.get(url || browser.baseUrl); | |||
| await browser.get(url || this.baseUrl); | |||
There was a problem hiding this comment.
So accessing the browser.baseUrl when it ends in a forward slash errors?
There was a problem hiding this comment.
No, but I want to be consistent 😁
| @@ -16,7 +17,7 @@ describe(browser.baseUrl, () => { | |||
| it(`should not redirect '${url}' (${i + 1}/${page.sitemapUrls.length})`, async () => { | |||
| await page.goTo(url); | |||
|
|
|||
| const expectedUrl = prependBaseUrl(url); | |||
| const expectedUrl = stripTrailingSlash(page.baseUrl + url); | |||
There was a problem hiding this comment.
Could we not use path.join(page.baseUrl, url) here? That would prevent double slashes.
There was a problem hiding this comment.
Even if we used canonical-path (to avoid http:\\foo\bar), path.join() would normalize http://foo to http:/foo 😁
@kara mentioned the caretaker gets them (in some form).
B...but...this is already pretty simple 😛 |
|
This merges cleanly to master, but not to patch. Can you create a separate PR for the patch. |
…ngular#25641) As part of the tests run in the CircleCI `aio_monitoring` job, we need to retrieve `sitemap.xml` from the site under test. Previously, the URL used to retrieve that contained a double-slash (`//`). At some point, Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail. This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes. PR Close angular#25641
|
Backported as #25820. |
…ngular#25641) As part of the tests run in the CircleCI `aio_monitoring` job, we need to retrieve `sitemap.xml` from the site under test. Previously, the URL used to retrieve that contained a double-slash (`//`). At some point, Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail. This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes. PR Close angular#25641
…25641) (#25820) As part of the tests run in the CircleCI `aio_monitoring` job, we need to retrieve `sitemap.xml` from the site under test. Previously, the URL used to retrieve that contained a double-slash (`//`). At some point, Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail. This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes. PR Close #25641 PR Close #25820
…ngular#25641) As part of the tests run in the CircleCI `aio_monitoring` job, we need to retrieve `sitemap.xml` from the site under test. Previously, the URL used to retrieve that contained a double-slash (`//`). At some point, Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail. This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes. PR Close angular#25641
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
As part of the tests run in the CircleCI
aio_monitoringjob, we need to retrievesitemap.xmlfrom the site under test. Previously, the URL used to retrieve that contained a double-slash (//). At some point,Firebase (which is used for hosting the site) stopped normalizing double-slashes to a single slash, causing the test to fail.
This commit fixes the problem by ensuring that the constructed URLs do not contain double-slashes.