RefugeRestrooms / refugerestrooms

REFUGE restrooms indexes and maps safe restroom locations for trans, intersex, and gender nonconforming individuals.
http://www.refugerestrooms.org
GNU Affero General Public License v3.0
888 stars 262 forks source link

Replace PhantomJS with Chromium #691

Closed mmx900 closed 2 months ago

mmx900 commented 3 months ago

Context

Digging the test failure of develop branch, I found that the project is still using old PhantomJS(with Poltergeist) which is outedated and doesn't support ES6 features. When I try to debug using page.driver.debug, I can see console errors including Unexpected token 'const'. As I remember there are many weird bugs in PhantomJS. So I just replaced that with Chromium/Cuprite/Ferrum. Now all tests are passed in my local environment with docker compose run -e "RAILS_ENV=test" web rake db:test:prepare spec. But I'm not sure why it was OK before https://github.com/RefugeRestrooms/refugerestrooms/commit/2c70ab90249274eab8ee80366b3deb693eeb8907.

Summary of Changes

Checklist

DeeDeeG commented 3 months ago

Thank you for this pull request. I'll take a look at it.

DeeDeeG commented 3 months ago

I'm not sure why it was OK before https://github.com/RefugeRestrooms/refugerestrooms/commit/2c70ab90249274eab8ee80366b3deb693eeb8907.

Fair question. I would guess something changed in one of our dependencies, perhaps the Google Maps library.

I ran a passing CI run of 2c70ab90249274eab8ee80366b3deb693eeb8907 back on the 10th of May 2023. (Passing run: https://app.travis-ci.com/github/RefugeRestrooms/refugerestrooms/builds/262958108). And then did a CI run with no code changes, to see if things were still in good shape, on the 4th of October 2023, which failed. (Failing run with no code changes 5 months later: https://app.travis-ci.com/github/RefugeRestrooms/refugerestrooms/builds/266379502). (This revealed another, separate issue with the old Travis-CI Xenial CI environment, which was fixed by updating Travis-CI to use a newer base OS. Fixed by: https://github.com/RefugeRestrooms/refugerestrooms/pull/683. But we still have the one failing spec, in our out of CI, even after fixing the Ubuntu Xenial CI issue.)

So, I suspect one or more of our many dependencies changed "out from under us," so to speak. We can't pin to a specific Google Maps library version anymore, only define the pace of updates as "weekly" or "quarterly", if I recall correctly. Since most of our other dependencies are locked in our yarn.lock or Gemfile.lock lockfiles, I suppose it would have to be something we can't lock down, such as the Google Maps library.

For reference, you can see our total CI build history here: https://app.travis-ci.com/github/RefugeRestrooms/refugerestrooms/builds.

DeeDeeG commented 3 months ago

Sorry for the delay, I've been working on getting CI running again for this repo, plus attending to some IRL things.

There is a Ferrum:TimeoutError when I try to run this PR through CI:

     # --- Caused by: ---
     # Ferrum::TimeoutError:
     #   Timed out waiting for response. It's possible that this happened because something took a very long time (for example a page load was slow). If so, setting the :timeout option to a higher value might help.
     #   /usr/local/bundle/gems/ferrum-0.14/lib/ferrum/browser/client.rb:46:in `command'

A fuller snippet of the log output:

Failures:
  1) restrooms preview can preview a restroom before submitting
     Failure/Error: visit "/"
     Ferrum::PendingConnectionsError:
       Request to http://127.0.0.1:41901/ reached server, but there are still pending connections: http://127.0.0.1:41901/
     # /usr/local/bundle/gems/ferrum-0.14/lib/ferrum/page.rb:124:in `rescue in go_to'
     # /usr/local/bundle/gems/ferrum-0.14/lib/ferrum/page.rb:113:in `go_to'
Rails 7.1 has deprecated the singular fixture_path in favour of an array
If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.
     # /usr/local/bundle/gems/cuprite-0.15/lib/capybara/cuprite/browser.rb:62:in `visit'
     # /usr/local/bundle/gems/cuprite-0.15/lib/capybara/cuprite/driver.rb:52:in `visit'
     # /usr/local/bundle/gems/capybara-3.39.2/lib/capybara/session.rb:280:in `visit'
     # /usr/local/bundle/gems/capybara-3.39.2/lib/capybara/dsl.rb:52:in `call'
     # /usr/local/bundle/gems/capybara-3.39.2/lib/capybara/dsl.rb:52:in `visit'
     # ./spec/features/restrooms_spec.rb:94:in `block (3 levels) in <top (required)>'
     # /usr/local/bundle/gems/webmock-3.12.2/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # Ferrum::TimeoutError:
     #   Timed out waiting for response. It's possible that this happened because something took a very long time (for example a page load was slow). If so, setting the :timeout option to a higher value might help.
     #   /usr/local/bundle/gems/ferrum-0.14/lib/ferrum/browser/client.rb:46:in `command'
1 deprecation warning total
Finished in 27.83 seconds (files took 2.09 seconds to load)
64 examples, 1 failure
Failed examples:
rspec ./spec/features/restrooms_spec.rb:93 # restrooms preview can preview a restroom before submitting
Randomized with seed 58963

Those are the logs I gathered from getting this PR running in GitHub Actions: https://github.com/DeeDeeG/refugerestrooms/actions/runs/8868660490/job/24348476133

Similar error in Travis CI: https://app.travis-ci.com/github/DeeDeeG/refugerestrooms/builds/270094473#L2761

mmx900 commented 3 months ago

Thanks for the detailed explanation. I've incresed the :timeout for Cuprite from the default of 5 seconds to 30 seconds to prevent the error. Now it seems to be fixed on both Github Action and Travis CI. However, there are still some Rubocop errors since https://github.com/RefugeRestrooms/refugerestrooms/commit/aa22bf77718b3bc9eef905acdb89805eb857ef24. I think I can fix them, but I'm not sure if this PR is right place to do it.