Skip to content

Hardcode chromedriver version until we get an upgrade that works#8942

Merged
soniaconnolly merged 3 commits intomainfrom
sonia-rails-helper-chrome-version
Aug 7, 2023
Merged

Hardcode chromedriver version until we get an upgrade that works#8942
soniaconnolly merged 3 commits intomainfrom
sonia-rails-helper-chrome-version

Conversation

@soniaconnolly
Copy link
Contributor

@soniaconnolly soniaconnolly commented Aug 4, 2023

Chromedriver versions are behind Chrome, so we need to hardcode the version until Chromedriver catches up.

This is affecting more and more devs who want to run feature specs, and it's a hassle to keep an edited rails_helper.rb around without committing it. We can revert this when we get a version of Chromedriver that works.

ETA: Looks like we need to upgrade the version of Chrome that CI uses to make this work. Is that an option?

Chromedriver versions are behind Chrome, so we need to hardcode the version until Chromedriver catches up.

[skip changelog]
@aduth
Copy link
Contributor

aduth commented Aug 7, 2023

ETA: Looks like we need to upgrade the version of Chrome that CI uses to make this work. Is that an option?

We probably could (should even?). I can't recall if that's as easy as just getting a "fresh" CI image SHA from devops (e.g. bottom of the build log here) to use in config, or if it needs something else.

Alternatively, we could maybe isolate it to local development, either using the ENV['CI'] flag, or explicitly catching the error and falling back?

require 'webdrivers'

begin
  Webdrivers::Chromedriver.latest_version
rescue Webdrivers::VersionError
  Webdrivers::Chromedriver.required_version = '114.0.5735.90'
end

CI is using an older version and there were issues when we tried to upgrade.
@soniaconnolly soniaconnolly force-pushed the sonia-rails-helper-chrome-version branch from d2e9142 to e0d9274 Compare August 7, 2023 15:26
@soniaconnolly
Copy link
Contributor Author

we could maybe isolate it to local development, either using the ENV['CI'] flag

That works much better, thanks! I thought about that too after walking away from the computer on Friday.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

# temporary fix for local development feature tests
# remove when we get a new working version of Chromedriver
if ENV['CI'] != 'true'
Webdrivers::Chromedriver.required_version = '114.0.5735.90'
Copy link
Contributor

Choose a reason for hiding this comment

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

I might expect to see this configuration someplace like support/capybara.rb, but not a big deal if it's short-lived.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. That works too. No need to reset it before each spec.

Copy link
Contributor

@timothy-spencer timothy-spencer left a comment

Choose a reason for hiding this comment

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

If this gets you going, LGTM! Longer term, we should move the IDP CI build over here so that you have direct control over it and don't need to hack in stuff like this.

@soniaconnolly soniaconnolly merged commit 489785f into main Aug 7, 2023
@soniaconnolly soniaconnolly deleted the sonia-rails-helper-chrome-version branch August 7, 2023 17:22
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.

3 participants