Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update Capybara #68

Merged
merged 4 commits into from
Jan 2, 2024
Merged

Update Capybara #68

merged 4 commits into from
Jan 2, 2024

Conversation

Nitemaeric
Copy link
Contributor

Context

In preparation for the capybara-screenshot gem, I found that the screenshot gem was not taking image screenshots. Ref: mattheworiordan/capybara-screenshot#211 (comment)

Upon investigation, I also found that we had a system spec not running javascript. This PR also addresses this issue.

Changes proposed in this pull request

  • Setup RSpec to refer to Capybara's current driver
  • Configure OmniAuth to use Rails' default logger and avoid RSpec results clutter
  • Add js: true in Javascript-dependent system specs
  • Use the --headless=new argument in the Selenium Chrome driver based on https://developer.chrome.com/docs/chromium/new-headless
  • Change https to http in Capybara host in system specs

Guidance to review

Try running bundle exec rspec locally.

Link to Trello card

https://trello.com/c/mzC6njCy/95-fix-javascript-dependent-system-specs

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Required environment variables have been updated or added to the Azure KeyVault

Screenshots

CleanShot 2024-01-02 at 17 12 24@2x

@Nitemaeric
Set OmniAuth logger to Rails logger
c8e2328 
This silences stdout messages while running rspec
@Nitemaeric
Update Capybara configuration
968a6fd 
Capybara's default chrome headless driver is :selenium_chrome_headless. Override that one.

Use --headless=new. See more: https://developer.chrome.com/docs/chromium/new-headless

`js: true` is a default supported option for Capybara. We only need to set the system tests to be driven by Capybara's current driver.
@Nitemaeric
Use js: true to enable javascript in js-dependent system spec
b2a311e
@Nitemaeric
Use http in tests to avoid TLS errors
7ed6815
@Nitemaeric Nitemaeric self-assigned this Jan 2, 2024
@Nitemaeric Nitemaeric enabled auto-merge (rebase) January 2, 2024 17:17
JamieCleare2525
JamieCleare2525 approved these changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@JamieCleare2525 JamieCleare2525 left a comment

Choose a reason for hiding this comment

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

Tested locally and seems to work.

@Nitemaeric Nitemaeric merged commit 77d5349 into main Jan 2, 2024
3 checks passed
@Nitemaeric Nitemaeric deleted the dd/fix-capybara branch January 2, 2024 17:17
@Nitemaeric
Copy link
Contributor Author

Fix Javascript-dependent System Specs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamieCleare2525 JamieCleare2525 JamieCleare2525 approved these changes

@ollietreend ollietreend Awaiting requested review from ollietreend

@CatalinVoineag CatalinVoineag Awaiting requested review from CatalinVoineag

@elceebee elceebee Awaiting requested review from elceebee

@gms-gs gms-gs Awaiting requested review from gms-gs

Assignees

@Nitemaeric Nitemaeric

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nitemaeric @JamieCleare2525