[rfc][icedtea-web] Refactor of LiveConnect Tests Version 2

[Andrew Azores]

On 06/05/2014 10:32 AM, Jie Kang wrote:
> Hello,
>
>>> ----- Original Message -----
>>>> Hello,
>>>>
>>>> This patch refactors the majority of LiveConnect Tests (JS to Java and
>>>> vice-versa) fixing the race condition between asynchronous JS and the
>>>> J-applet. Code format is standardized and deprecated code usage is
>>>> removed/replaced. Note that the file JSTest.js is duplicated for all
>>>> LiveConnect Tests. This is because every single test requires the same
>>>> function in JSTest.js to prevent the race condition from occurring.
>>>> However,
>>>> the test suite is not designed to easily have this code appear only once
>>>> while also having tests be properly separate (ie. every single test-case
>>>> should be able to run by itself without relying on another test case). If
>>>> anyone has any suggestions on how to better implement this please say.
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Jie Kang
>> I haven't looked over the whole patch yet, but as far as the Makefile
>> changes go the first thing I notice is that your new stamp's name is a
>> bit confusing. Why does it contain the word 'compile'? For example, look
>> at the stamp name 'copy-reproducers-resources'. Perhaps this new stamp
>> can be called just 'copy-shared-reproducers-resources' instead?
>>
> The naming was a mistake on my part. I originally intended to have shared resources include Java files as well hence the need for compilation but later on decided this wasn't feasible. I have renamed it to copy-shared-resources.
>
>> +	cp -r "$(REPRODUCERS_SHAREDSRCDIR)/resources/"*
>> $(REPRODUCERS_TESTS_SERVER_DEPLOYDIR)/shared/  ; \
>>
>>
>> Is there a particular reason that you have the asterisk outside of the
>> quotes in that first argument? Not wrong, just a little strange.
>> Secondly, you should probably quote the entire second argument. What
>> happens to your stamp right now if REPRODUCERS_TESTS_SERVER_DEPLOYDIR
>> gets redefined such that its name contains a space?
> Using cp "*" versus cp * has different end-result so putting * in the "" ends up not working. See:
> cp -R "$(REPRODUCERS_TESTS_SRCDIR)/$$which/$$dir/resources/"*  $(REPRODUCERS_TESTS_SERVER_DEPLOYDIR)/ ; \
> from 'stamps/copy-reproducers-resources.stamp'.

D'oh, of course. My mistake, sorry.

>
> I added the quotes to the last argument as suggested.
>
>> I don't know if Omair had anything in particular in mind for this
>> "shared resources" directory but this looks like it's on the right track
>> to me.
>>
>> Thanks,
>>
>> --
>> Andrew A
>>
>>
>
> Thanks
>
> Jie

This looks better, but it'll take some time for me to look through it 
all thoroughly.

Also, I should've caught this in the last mail I sent, but please be 
sure your ChangeLog entries adhere to the format. There should be a 
space between each * and the file names following it.

Thanks,

-- 
Andrew A