[rfc][icedtea-web] Makefile Reproducer Tests Patch

[Jiri Vanek]

On 09/18/2014 04:47 PM, Jie Kang wrote:
>
>
> ----- Original Message -----
>> On 09/17/2014 02:54 PM, Jie Kang wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> On 09/16/2014 10:36 PM, Jie Kang wrote:
>>>>> Hello,
>>>>>
>>>>>
>>>>> This patch modifies the makefile and causes it to only process (compile,
>>>>> copy, etc.) tests in the whitelist.
>>>>>
>>>>> This feature is greatly beneficial when running say
>>>>>
>>>>> 'make run-netx-dist-tests'
>>>>>
>>>>> which uses the whitelist to only run tests in the whitelist. Prior to
>>>>> this
>>>>> patch, though the tests ran were filtered by the whitelist, all tests
>>>>> were
>>>>> processed (compiled, resources moved, etc.). This wastes a large amount
>>>>> of
>>>>> time processing tests that aren't run.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>>
>>>>> Regards.
>>>>>
>>>>
>>>> By doing so, the dependences of reproducer may not be compiled and
>>>> installed.
>>>> Is it what you wont?
>>>
>>> Hello,
>>>
>>>
>>> If a reproducer depends on another reproducer than I think it should be up
>>> to the dev to deal with that appropriately through documentation, etc. In
>>> the best-case scenario, no test, including reproducers, should have any
>>> dependency on another test.
>>>
>>> Maybe we can add this functionality as a separate option? Then test-runner
>>> can choose to compile+install all tests, or compile+install only tests in
>>> the whitelist. This might be a good compromise.
>>>
>>>
>> I would say no.
>>
>> The author of whitelist modifications should be aware of his depnedencies.
>> They are moreover easily
>> grep-able  from jnlp.
>
> Hello,
>
>
> Just some clarification and comments :)
>
>
> I think this is also what I meant when I said dev; The person who changes whitelist (probably a developer, but I guess not always) should know his dependencies.

Sure. But consider case, that you are actually hacking some old reproducr:( Even you have missed the 
possibility to grep jnlp fiels for depndencies. So imagine someone less expiriecned.

Also there may be some strange cornercase reproducers, where the jars are laoded dynamicaly, so it 
really may not be clear.
>
>>
>> So on one side I would be happy for this change. On second side I'm
>> hesitating. Imagine test testing
>> class not found, loading some remote depndence.  Then it will not clear if it
>> failed because of
>> whitelist, or becasue of bug. And imagine you are fresh to itw and encounter
>> this.
>>
>> The configure option seems to be good compromise.  BY default all will be
>> compiled+isntalled. On
>> demand, only whitelist.
>
> Okay. Sounds good, I will work on this.
>
>>
>> if you wont you may (read must in this case) add this configure option as
>> another changeset.
>
> I am a little confused here. When you say another changeset, do you mean I should push the current change and then add a new one on top for the 'configure option'? Or create new change and discard the current one?

Yes. If you wont, you may push as it is, but you must add option later.
Or you may wait, and post it as one bigger changeset. TBH I prefer the first.
>
>>
>> The only request for now is that "*" must keep working :)
>
> * does work for this patch :)

great. The daily report will tell;)
>
>
> Thanks for the review!!
>
>>
You are welcomed :)


J,