makandra / geordi

Collection of command line tools used in our daily work with Ruby, Rails and Linux.
https://makandra.com/
MIT License
104 stars 16 forks source link

chromedriver_update blocks "geordi rspec" from working offline #204

Closed makmic closed 7 months ago

makmic commented 10 months ago

I recently tried to run tests offline with the geordi rspec command. It yielded the following exception:

~/../net/http.rb:1271:in `initialize': Failed to open TCP connection to googlechromelabs.github.io:443 (Connection refused - connect(2) for "googlechromelabs.github.io" port 443) (Errno::ECONNREFUSED)
    from ...geordi-9.6.1/lib/geordi/chromedriver_updater.rb:79:in `fetch_response'
    from ...geordi-9.6.1/lib/geordi/chromedriver_updater.rb:104:in `chromedriver_download_data'
    from ...geordi-9.6.1/lib/geordi/chromedriver_updater.rb:115:in `latest_version'
    from ...geordi-9.6.1/lib/geordi/chromedriver_updater.rb:15:in `run'
    from ...geordi-9.6.1/lib/geordi/commands/chromedriver_update.rb:19:in `chromedriver_update'
    from ...thor-1.3.0/lib/thor/command.rb:28:in `run'
    from ...thor-1.3.0/lib/thor/invocation.rb:127:in `invoke_command'
    from ...thor-1.3.0/lib/thor.rb:527:in `dispatch'
    from ...thor-1.3.0/lib/thor/invocation.rb:116:in `invoke'
    from ...geordi-9.6.1/lib/geordi/cli.rb:23:in `invoke_geordi'
    from ...geordi-9.6.1/lib/geordi/commands/rspec.rb:20:in `rspec'
    from ...thor-1.3.0/lib/thor/command.rb:28:in `run'
    from ...thor-1.3.0/lib/thor/invocation.rb:127:in `invoke_command'
    from ...thor-1.3.0/lib/thor.rb:527:in `dispatch'
    from ...thor-1.3.0/lib/thor/base.rb:584:in `start'
    from ...geordi-9.6.1/exe/geordi:12:in `block in <top (required)>'
    from ...geordi-9.6.1/lib/geordi/util.rb:14:in `installing_missing_gems'
    from ...geordi-9.6.1/exe/geordi:7:in `<top (required)>'
    from ~/.rbenv/versions/3.2.2/bin/geordi:25:in `load'
    from ~/.rbenv/versions/3.2.2/bin/geordi:25:in `<main>'

I was surprised that geordi connects to googlechromelabs.github.io every time I run tests, but looking at the source code it makes sense. Two suggestions:

nhasselmeyer commented 10 months ago

It should only connect to the internet / try the update when google-chrome --version yields another major version than chromedriver --version

We intentionally changed that behavior a year ago when a chromedriver bug was fixed in later release, but within the same chrome major version. To be able to automatically update to the version with the bugfix, we now try to update even if there already is a chromedriver for the given chome major version.

makmic commented 10 months ago

It should only connect to the internet / try the update when google-chrome --version yields another major version than chromedriver --version

We intentionally changed that behavior a year ago when a chromedriver bug was fixed in later release, but within the same chrome major version. To be able to automatically update to the version with the bugfix, we now try to update even if there already is a chromedriver for the given chome major version.

Okay, I get that. But then we should only warn about the failed update check and continue running tests in my case - right?

brunosedler commented 10 months ago

This will be fixed in https://github.com/makandra/geordi/pull/206