[rfc][icedtea-web] Resource/ResourceTracker clean up

[Omair Majid]

* Andrew Azores <aazores at redhat.com> [2014-05-07 12:02]:
> >ugh... I would like to stop this being pushed until the unitttest for
> >*original* behavior are done. And then they are adapted to new "behavior".
> >This part of itw is lacking the tests,  so here is actually no trace if
> >behvaiour was preserved.
> >
> >Otherwise the patch is ok.
> 
> Alright. I've attached tests for Resource#status and for
> ResourceTracker.selectByFlag in their current state. They can be fairly
> easily reworked for the EnumSet patch once you verify that the tests are
> sufficiently guarding against behaviour changes (or find that they aren't
> and I fix it so they are!). These new tests should apply to HEAD and all
> pass currently.

Looks okay to me. Some of utility functions in the test class are
unused. I assume that's intentional; please remove them if it's a
mistake.

> +++ b/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java
> @@ -0,0 +1,155 @@

> +    private static final Version VERSION1 = new Version("1.0");

This is used in one one method, and the actual version is insignificant.
Maybe call it SOME_VERSION or A_VERSION and move it to the method
itself?

> +    @Test
> +    public void testNewResourceIsUninitialized() throws Exception {
> +        Resource res = getDummyResource("NewResource");
> +        assertTrue("Resource should not have had any status flags set", hasFlag(res, UNINITIALIZED));

This test reads a little funny. Normally names like 'dummy' (and 'stub',
'fake', 'temp') are for an object which is mostly incidental to the
test. So a function named `getDummyResource` that returns the very thing
this test is looking at seems a little odd. Maybe call it
`createResource` or something?

> +    private static Resource getDummyResource(String testName) throws MalformedURLException {
> +        URL dummyUrl = new URL("http://example.com/applet" + testName + ".jar");
> +        return Resource.getResource(dummyUrl, VERSION1, UpdatePolicy.ALWAYS);

This doesn't create a global, right? Calling this method with the same
argument multiple times returns new objects, right?

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681