Skip to content

[6.1.x] test(docs-infra): fix double-slash in URL of aio_monitoring test #25820

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

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Sep 5, 2018

Backporting #25641 to 6.1.x. This required backporting the first commit of #19795, which will eventually be backported to 6.1.x anyway.

@gkalpak gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer comp: docs-infra labels Sep 5, 2018
@gkalpak
Copy link
Member Author

gkalpak commented Sep 5, 2018

The current CI failures are "inherited" from upstream. (See #25827.)

`%%DEPLOYMENT_HOST%%` has been assumed to be the host prefix for sitemap
URLs since bf29936, but afaict this was never the case.

PR Close angular#19795
…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
@gkalpak gkalpak force-pushed the 6.1.x-fix-docs-infra-aio_monitoring-CI-job branch from 6e61162 to 98663d2 Compare September 6, 2018 05:42
@mary-poppins
Copy link

You can preview 98663d2 at https://pr25820-98663d2.ngbuilds.io/.

@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Sep 6, 2018
IgorMinar pushed a commit that referenced this pull request Sep 6, 2018
`%%DEPLOYMENT_HOST%%` has been assumed to be the host prefix for sitemap
URLs since bf29936, but afaict this was never the case.

PR Close #19795

PR Close #25820
IgorMinar pushed a commit that referenced this pull request Sep 6, 2018
…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
@IgorMinar
Copy link
Contributor

can you please rebase this PR? thanks

@IgorMinar IgorMinar added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels Sep 6, 2018
@gkalpak
Copy link
Member Author

gkalpak commented Sep 7, 2018

Apparently, this has been merged already as ecc3406 and 62f4ea5. Closing.

@gkalpak gkalpak closed this Sep 7, 2018
@gkalpak gkalpak deleted the 6.1.x-fix-docs-infra-aio_monitoring-CI-job branch September 7, 2018 08:30
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants