Skip to content

[0.14.3] Support custom save_path option #217

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

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

singhprd
Copy link
Contributor

@singhprd singhprd commented Nov 15, 2022

Allows paths other than Capybara.save_path to be specified

This PR allows a custom save_path option to be configured, instead of defaulting to the Capybara.save_path.

The motivation is wanting to specify an ephemeral directory for downloads (PDFs, CSVs etc) which is cleared between tests, but keep other items saved to the Capybara.save_path (like screenshots) as CI artifacts.

This is especially useful when running multiple specs on a single instance. Here is how we separate the downloads using this patch:

module Downloads
  def downloads_path
    Rails.root.join("log", "capybara", "downloads", ENV.fetch("TEST_ENV_NUMBER", "1"))
  end
end

...
...

cuprite_options = {
  process_timeout: 60,
  ...
  save_path: Downloads.downloads_path,
}

...

Capybara.register_driver :cuprite_chrome do |app|
  Capybara::Cuprite::Driver.new(app, options)
end

Original PR: #192 (this one has just rebased against 0.14.3)

@singhprd singhprd changed the title Support custom save_path option [0.14.3] Support custom save_path option Nov 15, 2022
Allows paths other than Capybara.save_path to be specified
@singhprd singhprd force-pushed the custom-save-path-0.14.3 branch from b987718 to 12e34ec Compare November 15, 2022 15:42
@singhprd singhprd marked this pull request as ready for review November 15, 2022 15:44
@singhprd
Copy link
Contributor Author

Hey @route! 👋

Hope you're doing well. Thanks for your fantastic work maintaining this Gem, it's super helpful and really appreciated! 🙇

If you get some time, would you be able to evaluate if this is able to be merged in please, hopefully it helps someone else out and I can retire our fork 🍴

Thanks!
Pete

@singhprd
Copy link
Contributor Author

Related issue: #198

@route
Copy link
Member

route commented Nov 15, 2022

The tests are all failing

@singhprd
Copy link
Contributor Author

singhprd commented Nov 16, 2022

Ah, thanks for approving them to run. I'll take a look!

@route route merged commit 9706b5a into rubycdp:master Nov 16, 2022
@singhprd
Copy link
Contributor Author

singhprd commented Nov 16, 2022

Thanks @route! - 🎉 😃

I was just about to ask you to re-run the tests as they were passing on my fork, but you've already done it 🚀 🪨

And have a great rest of your day! 🌤️

@route
Copy link
Member

route commented Nov 16, 2022

Thank you! You too!

cbliard added a commit to opf/openproject that referenced this pull request Sep 19, 2024
rubycdp/cuprite#217 has been merged, `save_path`
is correctly taken into account now.
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