[rfc][icedtea-web][policyeditor] Unit tests fix after last PolicyEditor async IO patch

[Andrew Azores]

On 06/03/2014 05:23 PM, Omair Majid wrote:
> * Andrew Azores <aazores at redhat.com> [2014-06-02 17:08]:
>> There's a race between the unit tests and PolicyEditor instances that is
>> causing several PolicyEditor tests to sporadically fail, due to the tests
>> asserting certain conditions about the contents of the policy file model
>> before it has actually been opened and parsed. This patch fixes the race by
>> adding a "performingIO" field to the editor, which is simply marked true
>> immediately before starting an IO operation and marked false immediately
>> after completing it. The test cases then poll the editor every 50ms, waiting
>> for it to complete its IO, before performing the actual tests.
> I am not sure I like this. Wouldn't it be better if the test was more
> controlled and predictable? I am thinking that the test should avoid
> executing thing async. Instead, it should get hold of a Runnable or
> something and then execute it in the current thread. This makes sure
> exceptions are caught (side note: SwingWorker and SwingUtilities can
> silently swallow exceptions [1]) and also avoids races and makes things
> run in a more predictable location.
>
> Thanks,
> Omair
>
> [1] http://jonathangiles.net/blog/?p=341
>

The test itself isn't doing anything async, it's PolicyEditor doing it 
after the SwingWorker patch. The tests used to expect that after 
instantiating a PolicyEditor, once the constructor returned, the 
editor's model of the policy file would already be ready. Now that the 
file loading is done async that assumption doesn't hold, and it's 
possible for the editor to not have parsed the policy file by the time 
the tests run. There are already tests for the parsing stage on its own, 
so extracting out the logic into a tests-accessible Runnable would 
mostly just be duplicating the standalone parser tests, whereas the 
tests affected here are meant to test that the editor opens with a valid 
state and handles exposing information about its internal model in a 
reasonable way.

Thanks,

-- 
Andrew A