Force url parameter in visit method with to_s

[alexpeachey]

The visit method takes a url as a parameter and in checking for special cases it performs url.to_s. However if no special cases apply, it does not perform to_s.

As someone who would like to send an arbitrary object capable of producing a url from to_s I feel adding consistency to the treatment of the parameter is beneficial.

[twalpole]

This changes the current driver api - and therefore isn't going to be considered before v3

[alexpeachey]

I don't see how this changes the driver api. The docs around the visit method specify it is supposed to receive a string and that it expects a string and in all "special" cases it is forced into a string.

The driver visit method also expects a string and in fact, RackTest, makes a very big assumption that it is a string because it uses URI.parse on it which fails when it isn't a string. In the Selenium driver if it isn't handed a URI it uses URI.parse.

If anything, this change makes the visit method more robust as it ensures the driver does actually get a string instead of only sometimes getting a string.

The entire capybara test suite passes with this change.

[twalpole]

Yes -- the capybara test suite does pass, however there are other drivers that use capybara -- changing the behavior of parameters currently passed to those drivers isn't going to happen until the next major revision - v3

[alexpeachey]

Ok, thank you. I can understand that even though the contract and documentation on the visit method specify it should be a string, it doesn't mean that people who wrote drivers actually adhered to the documented API and may have just adhered to the behavior of the method.

So changing this on the next major version makes sense.

[abotalov]

@twalpole I disagree with you.

Previously Capybara could sent to driver's visit method either String or not String. If user sends parameter of String type, then String will be sent to driver.

After this PR (or #1357) String is sent to driver's visit method (and it was supported previously). So it doesn't invoke any changes neither at driver, nor at user side.

[twalpole]

@abotalov I'll go through it again - although alex understood it so I think I explained my reasoning fine above. As you said - previously Capybara could send to drivers visit method either a String or not a String (ie a URI could be passed through untouched, even though undocumented in some cases) and I wasn't willing in a minor version to change that just so the user didn't have to call #to_s on an arbitrary object in case an existing driver was actually using that undocumented "feature". The visit method has since been changed to actually fix a bug - which I consider more justifiable than just for ease of use - and so it may now be fine to revisit this issue. As it is the documentation for the visit method specifies that a String must be passed in, so any pull request for this issue will need to have that documentation updated to show that an ojbect that responds to #to_s would be acceptable as well

[twalpole]

PR #1357 covers the same issue