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

Jump to bottom

Fix visiting urls with non-http(s) schemes #1339

Merged
merged 1 commit into from Jun 20, 2014
Merged

Fix visiting urls with non-http(s) schemes #1339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/capybara/session.rb
View file
Open in desktop
Expand Up @@ -206,12 +206,15 @@ def visit(url)

@touched = true

if url !~ /^http/ and Capybara.app_host
url_relative = URI.parse(url.to_s).scheme.nil?

if url_relative && Capybara.app_host
url = Capybara.app_host + url.to_s
url_relative = false
end

if @server
url = "http://#{@server.host}:#{@server.port}" + url.to_s unless url =~ /^http/
url = "http://#{@server.host}:#{@server.port}" + url.to_s if url_relative
Copy link
Member

Choose a reason for hiding this comment

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

this will create an invalid url if both Capybara.app_host and @server are set

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@server is set in initialize method when Capybara.run_server and @app and driver.needs_server? is true. It seems to me Capybara doesn't support a case when both app and app_host are set, does it?

Copy link
Member

Choose a reason for hiding this comment

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

app_host would override @server because the code prepended app_host to the url and then in the if @server block the url would already begin with ^http. A way to fix is just to set url_relative=false after prepending app_host

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems this bug existed before this PR too. And I'm not sure what Capybara should append if both server and app_host are set. Also it should be documented what takes precedence if both of them are set.

Copy link
Member

Choose a reason for hiding this comment

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

it didnt exist before because once Capybara.app_host was appened (http://www.google.com for instance) then it matched /^http/ so "http://#{@server.host}:#{@server.port}" was not prepended in the if @server section

Copy link
Member

Choose a reason for hiding this comment

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

Also - Capybara.app_host needs to take precendence, otherwise its useless -- Its handy for testing multi-tenant by subdomain systems.

Copy link
Member

Choose a reason for hiding this comment

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

It is documented in the Calling remote servers section of the readme - "Normally Capybara expects to be testing an in-process Rack application, but you can also use it to talk to a web server running anywhere on the internet, by setting app_host:" .... "By default Capybara will try to boot a rack application automatically. You might want to switch off Capybara's rack server if you are running against a remote application: Capybara.run_server = false"

ie -- app_host has precendence -- and you may want to turn off server if you're using it (or may not - either way app_host has precendence)


if Capybara.always_include_port
uri = URI.parse(url)
Expand Down
5 changes: 5 additions & 0 deletions lib/capybara/spec/session/visit_spec.rb
View file
Open in desktop
Expand Up @@ -33,6 +33,11 @@
end.to raise_error(TestApp::TestAppError)
end

it "should be able to open non-http url", requires: [:about_scheme] do
@session.visit("about:blank")
@session.assert_no_selector :xpath, "/html/body/*"
end

context "when Capybara.always_include_port is true" do

let(:root_uri) do
Expand Down
3 changes: 2 additions & 1 deletion spec/dsl_spec.rb
View file
Open in desktop
Expand Up @@ -11,7 +11,8 @@ class TestClass
:frames,
:windows,
:server,
:hover
:hover,
:about_scheme,
]

RSpec.describe Capybara::DSL do
Expand Down
3 changes: 2 additions & 1 deletion spec/rack_test_spec.rb
View file
Open in desktop
Expand Up @@ -10,7 +10,8 @@ module TestSessions
:frames,
:windows,
:server,
:hover
:hover,
:about_scheme,
]

RSpec.describe Capybara::Session do
Expand Down