Make `be_url` stricter
Summary
There is a RSpec helper be_url
that has the following content:
# frozen_string_literal: true
RSpec::Matchers.define :be_url do |_|
match do |actual|
URI.parse(actual) rescue false
end
end
This seems good, but it probably doesn't do what we want, since the URI syntax includes things without schemes, undotted domains, etc. For example:
irb(main):003:0> URI.parse('foo')
=> #<URI::Generic foo>
irb(main):004:0> URI.parse('wibble://foo')
=> #<URI::Generic wibble://foo>
irb(main):005:0> URI.parse('baz?')
=> #<URI::Generic baz?>
All these very dubious strings validate successfully as URIs.
Improvements
Look at every place this matcher is used, and decide if we should not be verifying more invariants. For instance, we can easily verify if a URI is http addressable by casing on the class. For instance:
irb(main):006:0> URI.parse('http://some.domain')
=> #<URI::HTTP http://some.domain>
So the following might help us:
case URI.parse(str)
when URI::HTTP, URI::HTTPS
true
when URI::FTP
do_we_want_ftp?
else
false
end
This would allow us to have better tests that actually check the things we want them to.
Risks
This could break a bunch of tests.
Involved components
Not many thankfully: git grep only comes up with two hits in the spec directory, and one of them is the matcher itself:
$ git grep be_url spec/
spec/services/chat_names/authorize_user_service_spec.rb: is_expected.to be_url
spec/support/matchers/be_url.rb:RSpec::Matchers.define :be_url do |_|