Skip to content

ci: pin ChromeDriver to a version compatible with docker image's Chrome - #28494

Closed
gkalpak wants to merge 3 commits into
angular:masterfrom
gkalpak:ci-repin-ChromeDriver
Closed

gkalpak wants to merge 3 commits into
angular:masterfrom
gkalpak:ci-repin-ChromeDriver

Conversation

@gkalpak

@gkalpak gkalpak commented Feb 1, 2019

Copy link
Copy Markdown
Member

PR Type

  • CI related changes

Does this PR introduce a breaking change?

  • Yes
  • No

What is the current/new behavior?

By default, webdriver-manager update will download the latest ChromeDriver version, which might not be compatible with the Chrome version included in the [docker image used on CI], causing CI failures.
Previously, we used to pin the ChromeDriver version on CI in ngcontainer's Dockerfile. This was accidentally broken in #26691, while moving from ngcontainer to default CircleCI docker images.

This commit fixes the issue by pinning ChromeDriver to a known compatible version.

@gkalpak
gkalpak requested review from a team February 1, 2019 21:55
@gkalpak gkalpak added type: bug/fix area: build & ci Related the build and CI infrastructure of the project action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release risk: low labels Feb 1, 2019
@ngbot ngbot Bot added this to the needsTriage milestone Feb 1, 2019
@gkalpak
gkalpak force-pushed the ci-repin-ChromeDriver branch from c77b6e6 to 04a9e31 Compare February 1, 2019 22:27
@gkalpak
gkalpak force-pushed the ci-repin-ChromeDriver branch 3 times, most recently from a9e46d0 to de88fcc Compare February 1, 2019 23:07
@gkalpak gkalpak added state: WIP and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 1, 2019
@gkalpak
gkalpak force-pushed the ci-repin-ChromeDriver branch from 8c1754c to b787f64 Compare February 1, 2019 23:59
By default, `webdriver-manager update` will download the latest
ChromeDriver version, which might not be compatible with the Chrome
version included in the [docker image used on CI], causing CI failures.
Previously, we used to pin the ChromeDriver version on CI in
[ngcontainer's Dockerfile][2]. This was accidentally broken in angular#26691,
while moving from ngcontainer to default CircleCI docker images.

This commit fixes the issue by pinning ChromeDriver to a known
compatible version.

[1]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/.circleci/config.yml#L16
[2]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/tools/ngcontainer/Dockerfile#L63
@gkalpak
gkalpak force-pushed the ci-repin-ChromeDriver branch from b787f64 to f29b086 Compare February 2, 2019 00:00
@gkalpak gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Feb 2, 2019
@matsko matsko added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 2, 2019

@alexeagle alexeagle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would have been a smaller change to continue naming it $CHROMEDRIVER_VERSION_ARG

ideally keep each PR to just changing one thing rather than add refactorings like --progress=false

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Feb 2, 2019
matsko pushed a commit that referenced this pull request Feb 2, 2019
matsko pushed a commit that referenced this pull request Feb 2, 2019
…me (#28494)

By default, `webdriver-manager update` will download the latest
ChromeDriver version, which might not be compatible with the Chrome
version included in the [docker image used on CI], causing CI failures.
Previously, we used to pin the ChromeDriver version on CI in
[ngcontainer's Dockerfile][2]. This was accidentally broken in #26691,
while moving from ngcontainer to default CircleCI docker images.

This commit fixes the issue by pinning ChromeDriver to a known
compatible version.

[1]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/.circleci/config.yml#L16
[2]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/tools/ngcontainer/Dockerfile#L63

PR Close #28494
@matsko matsko closed this in 814ee26 Feb 2, 2019
matsko pushed a commit that referenced this pull request Feb 2, 2019
matsko pushed a commit that referenced this pull request Feb 2, 2019
…me (#28494)

By default, `webdriver-manager update` will download the latest
ChromeDriver version, which might not be compatible with the Chrome
version included in the [docker image used on CI], causing CI failures.
Previously, we used to pin the ChromeDriver version on CI in
[ngcontainer's Dockerfile][2]. This was accidentally broken in #26691,
while moving from ngcontainer to default CircleCI docker images.

This commit fixes the issue by pinning ChromeDriver to a known
compatible version.

[1]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/.circleci/config.yml#L16
[2]: https://github.com/angular/angular/blob/bfd48d156d5663cc49d1ec40c2d73ff76a2fe62f/tools/ngcontainer/Dockerfile#L63

PR Close #28494
@filipesilva

Copy link
Copy Markdown
Contributor

In the CLI we ended up using puppeteer to avoid being subject to specific CI browser versions (angular/angular-cli@bf56630).

@gkalpak
gkalpak deleted the ci-repin-ChromeDriver branch February 4, 2019 08:19
@angular-automatic-lock-bot

Copy link
Copy Markdown

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: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risk: low target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants