[rfc][icedtea-web] Fix CacheReproducerTest

[Jiri Vanek]

On 08/31/2013 12:06 AM, Jacob Wisor wrote:
> Hello,
> 
> if I may way in here...

Generally spoken good idea to use the map isntead of default location. Reason Why I not suggested it
is that I'm not sure how DeploymentConfiguration  will work when "misused" like this.  However
worthy to try.

> 
> "Andrew Azores"<aazores at xxxxxxxxxx> wrote:
>> On 08/30/2013 11:32 AM, Omair Majid wrote:
>>> On 08/30/2013 11:20 AM, Andrew Azores wrote:
>>>> On that note - Jiri, wouldn't using the value from Defaults not take
>>>> into account the user changing the cache location using itweb-settings?
>>>> Perhaps it makes more sense to use DeploymentConfiguration here to
>>>> ensure that the reproducer test is actually testing using the cache dir
>>>> that the user expects to use as a cache dir? This way if you've never
>>>> changed the cache location in itweb-settings then you get the same path
>>>> as taking it from Defaults would give you, but it does still also
>>>> respect if you've changed the cache location.
>>> This is sort of hand-wavy, but it might be a good idea to keep
>>> reproducers predictable. When the reproducers are run, they should use a
>>> 'known' configuration and check for exact paths.
> 
> I always assumed reproducers to be unpredictable on any random system by definition?

I would guess this can happen. Although number of reproducers depending on configuration directly is
very few.

> 
> I may be wrong here, but in my understanding a reproducer is not a general test, it is a special case or kind of test. A reproducer tests for or against a specific configuration and/or specific system and hence is allowed to fail on some systems. Imho a reproducer should not cover default configuration tests. It just gives a hint on what to fix to make an application or program work on a specific system or with a specific configuration. That said, a reproducer's results are per se unpredictable by definition. They are of course predictable with a specific configuration and/or on a specific system.

Unluckily historical true. I think now its a bit better.
> 
> Consequently imho testing for the default configuration should be put into a general test that must succeed on any system. Any specific configuration tests should be put into a reproducer that is allowed to fail on some systems (until the app/program has been adapted).

Yes, the testing on default configuration is the proffered way - mostly from historical reasons,
when framework was under development and custom configs  were not possible.

When you check newer reproducers they are stable, are running on clean configuration, and clean uo
after themselves too. Also they are testing various settings - both custom and defaults.



>>> A mistake in code (a
>>> typo in one of the configuration defaults, for example) should be
>>> detected by the reproducers.

Not easy task here. But I agree taht tets should not be depending on actual configuration. As I
mentioned. Newer tests are fighting with this a bit, and probably general solution like annotation
@itw-test (params like config, map, cleanCache...) which will setup clean default/custom environment
and then clean up after it may be worthy.


>>>
>>> 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.
>>>
>>> Thanks,
>>> Omair
>> 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). 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. 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. Speaking of which, perhaps there should be a
>> way within that GUI to reset the cache location to default?
> 
> Then the question is, what is the default? The default seems to be somewhat dynamic, that is system dependent, maybe even configuration dependent. But for the sake of simplicity, let's assume DeploymentConfiguration can provide a default configuration. I do not see anything speaking against a "reset to default" button, but I also do not see any real reason for it.
> 
> (1) does not seem to make sense because it actually should have been covered by a test.
> (2) Seems to make sense only if a non-default configuration has been already created (and it _must_ have been created). Perhaps creating some randomly generated configurations or special cases, like folder names with known invalid characters on some file systems, or embarrassingly long file and folder names would add any value?
> Sorry, but (3) does not make sense at all because it effectively does not test anything except for "not having the default configuration".
> 

Although I agree with you, I'm hesitating to touch reproducers somehow more - as they are serving
well. Andrew A. is looking into them so he may have different opinion - he already refactored some
parts of code for more  happy live of us.

J.