Arthur,
I like the way this is turning out.
> On 26 Mar 2019, at 12:05, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> ...
> 2. B6890349.java
>
> Using URI.toURL() in this test will change the nature of
> the test. I believe that in this specific case using the
> multi-arg URL constuctor should be preferred.
> The reason is that URI as per its specification encodes
> illegal characters, while URL as per its specification,
> does not. So going through URI here will lead to a
> different URL than the one that the test is testing.
>
> I see from your mail that you have noticed the difference :-)
>
> For this one test I'd suggest to make an exception and
> change the URL construction to:
>
> URL u = new URL("http",
> InetAddress.getLoopbackAddress().getHostAddress(),
> port,
> "/foo\nbar");
>
> Even if we get to deprecating the URL constructors in the
> future this would still be a valid case for using them in
> a test.
Right. The above accepting-illegal-characters behaviour is one of
the reasons, non-test code, should avoid the URL constructors.
> 3. on the 'nit' side it's a bit strange to see
> the sequence of calls `.host(..).path(..).port(..)`,
> as `.host(..).port(..).path(..)` would feel
> natural - but don't feel obliged to make this change.
URI component-wise ordering makes the code a little easier to
read. I’d just do it.
I dislike noisy tests, but it would be helpful for these tests to print
the URL that has been constructed ( System.out.println("URL: " + url) ).
It will make it easier to verify the test's behaviour on different
system configuration ( I usually just look in the jtr file )
-Chris.
