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.

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

Add save_and_open_page helper to IntegrationTest #49267

Merged

Conversation

JoeDupuis
Copy link
Contributor

@JoeDupuis JoeDupuis commented Sep 13, 2023

save_and_open_page is a capybara helper that lets developers inspect the status of the page at any given point in their tests. This is helpful when trying to keep a short feedback loop while working on a test.

This change adds a similar helper with a matching signature to integration tests.

20230913155205_0fccdf93e5.mp4

Motivation / Background

I have seen similar helpers defined in a number of projects in their test_helper.rb.
I figured this would be a good candidate to upstream as we already have the same helper for system tests.

Detail

Launchy is required to automatically open the dump in a browser, but the helper still works without. Without launchy it prints a warning along with the path to the dump for manual review.

Additional information

  • I wasn't sure where to put the changelog notice. I've put it above the Rails 7.1 release header as I assume this won't make the cut for the release
  • I did not add launchy to the Gemfile template. Maybe I should?

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot bot added the actionpack label Sep 13, 2023
@JoeDupuis JoeDupuis force-pushed the integration-test-save-and-open-page branch from 4667c25 to 334b7ba Compare September 13, 2023 23:10

def open_file(path)
require "launchy"
Launchy.open(path)
Copy link
Member

Choose a reason for hiding this comment

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

I think we would have to make this a direct dependency of actionpack, which we I'm not sure we want to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review.

I brought over the LoadError rescue from Capybara to prevent making it a hard dependency.

This is not a new pattern in Rails as it is already used to prevent ActiveSupport/ActionPack from depending on :

Would the "soft" dependency still be a problem?
It would be totally opt-in since it is never loaded unless you manually put it in your Gemfile.

@rafaelfranca rafaelfranca added the ready PRs ready to merge label Sep 29, 2023
@JoeDupuis JoeDupuis force-pushed the integration-test-save-and-open-page branch from 334b7ba to 01ba040 Compare October 4, 2023 01:44
@JoeDupuis JoeDupuis force-pushed the integration-test-save-and-open-page branch 2 times, most recently from 32d7283 to 6fbd276 Compare February 11, 2024 23:45
@JoeDupuis JoeDupuis force-pushed the integration-test-save-and-open-page branch 2 times, most recently from 610b286 to 78c544a Compare March 8, 2024 01:19
@zzak zzak added this to the 7.2.0 milestone Mar 8, 2024
`save_and_open_page` is a capybara helper that lets developers
inspect the status of the page at any given point in their
test. This is helpful when trying to keep a short feedback loop while
working on a test.

This change adds a similar helper with matching signature to
integration tests.
@rafaelfranca rafaelfranca force-pushed the integration-test-save-and-open-page branch from 78c544a to 7f9ce6f Compare April 18, 2024 20:44
@rafaelfranca rafaelfranca merged commit eae8f41 into rails:main Apr 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionpack ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants