[rfc][icedtea-web] Fix CacheReproducerTest

[Omair Majid]

On 08/30/2013 01:31 PM, Andrew Azores wrote:
> On 08/30/2013 11:32 AM, Omair Majid wrote:
>> Allowing a user's configuration to affect the test would surely make
>> things much less predictable - the opposite of what you want in a good
>> automated test.
>
> The problem is that as the test is now, it relies on the user leaving
> the cache location as default. If you use the default location, which it
> is set up to expect, then it works. But if you change the cache location
> with itweb-settings, then the reproducer tests all fail due to the cache
> location not existing. I can: (1) modify the reproducer to create the
> default cache location if it doesn't exist and then use it, or (2) have
> the reproducer use the DeploymentConfiguration-specified location
> whether or not that happens to be default, or (3) allow the reproducer
> to fail if you are not using the default cache location.
>
> I don't think it makes sense to choose (3), which is the current state
> of the reproducer, so the choice is really between (1) and (2). I see
> what you mean about having much more rigidly defined paths in the test
> for predictability, ie choosing option (1), but I think it also makes
> sense to use the configured cache path settings, ie option (2).

Can you explain how it makes sense?

The reason I prefer (1) is that it isolates what's under test from
random changes outside the environment. And that makes tests predictable
and reduces false positives. The more moving parts a test has, the
harder it is to know if a failure is really meaningful or a random
failure. Not to mention that you really want to hardcode many paths in
your tests - just to be able to catch typos in things like
DeploymentConfiguration.

Also, if we can set up and use separate configuration files for tests,
that makes it much easier to test the impact of configuration changes
(as well as test, say, things like configuration migration) safely.

> If we
> take the second option then this test also catches whether or not the
> DeploymentConfiguration settings are actually being modified by the
> itweb-settings GUI.

I thought the patch was for javaws? What you are suggesting sounds like
a separate test - for itweb-settings. And that would be nice to have
too, but as a separate test. If you mix it in this test, it's much
harder to identify from a test failure if it's caused by itweb-settings
or javaws.

> On the other hand, option (1) will tell you simply
> whether the caching mechanism is working correctly, regardless of if
> your particular cache configuration (ie cache location) works or not.
> Option (2) can emulate option (1) if you just set the cache location to
> default in itweb-settings.

In the short-term, 2 sounds okay to me. But I don't think it's the best
option long-term.

> Speaking of which, perhaps there should be a
> way within that GUI to reset the cache location to default?

Some way to reset things to defaults using itweb-settings sounds nice to
me (not sure how urgent/important it is, though).

Thanks,
Omair

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