teampoltergeist / poltergeist

A PhantomJS driver for Capybara
MIT License
2.5k stars 415 forks source link

Whitelist docs omit handy details #847

Closed neoeno closed 7 years ago

neoeno commented 7 years ago

Hello Team Poltergeist! Firstly, thanks for all your great work on this project!

Scenario

Today I wanted to block my tests from accessing external web services, so I could start narrowing down and possibly eliminating those cases.

First I tried this:

Capybara.register_driver :poltergeist do |app|
  Capybara::Poltergeist::Driver.new(app, {
    url_whitelist: []
  })
end

It took me a little while to figure out this does nothing. Poltergeist treats an empty array as if the option isn't specified at all.

So I tried this:

Capybara.register_driver :poltergeist do |app|
  Capybara::Poltergeist::Driver.new(app, {
    url_whitelist: ["notarealwebsite.com"]
  })
end

Which blocked everything — including localhost. Easier to figure out, so I allowed localhost like this:

Capybara.register_driver :poltergeist do |app|
  Capybara::Poltergeist::Driver.new(app, {
    url_whitelist: ["http://127.0.0.1:*"]
  })
end

Which I found by copying from #828. Before that, I suspected we might be dealing with patterns, but the docs hadn't told me this directly. In fact it's just "*" or "?" that's allowed and everything else is escaped. Also it's a substring match, so "github.com" will also match "api.github.com".

 Proposed solution

I'd like to make this a bit easier to set up. Here's what I'd like to do:

  1. Improve the README to clarify that an empty whitelist has no effect.
  2. Improve the README to clarify that most users will want to whitelist localhost
  3. Improve the README to clarify the pattern matching behaviour (any substring is allowed, you can use * and ?)

Let me know if that sounds good to you and I'll go ahead and submit a PR.

twalpole commented 7 years ago

@neoeno Not sure if you get notified when people 👍 your issues, but documentation improvements are always appreciated.

twalpole commented 7 years ago

Closing this issue due to lack of response - If/when a PR is submitted we can continue any needed discussion