[fyi][icedtea-web]Refactoring of reproducers as agreed in April

[Jiri Vanek]

On 06/29/2012 05:58 PM, Omair Majid wrote:
> On 06/29/2012 08:50 AM, Jiri Vanek wrote:
>> Hi! It looks to me that we have agreed on first five patches (2x hg mv,
>> extracting of tests and inner clases, adapted makefile and added
>> headers). I would like to push them (it is getting hard to maintain
>> those set of changsets).
>
> Fair enough. Just one request, could you ensure that every changeset is
> buildable? You should probably combine the one doing the hg rename with
> the ones doing the makefile changes to update the directory name. It
> doesnt really matter for now whether each changeset is buildable, if you
> pull all at once, but if we ever have to do a 'hg bisect' (or otherwise
> dig through history to identify a problem), a changeset that wont build
> can be painful. You don't have to post a separate patch review for this.

Kept. Both hg move tasks and exctracting of inner classes included in adapted makefile changsett.
>
>> Your next two concerns (netx on classapth and renaming) I would like to
>> provide as two  separate changsets, as it is not critically bounded. Se
>> my comments lower to the renaming patch as you wanted and get rid of
>> unnecessary netx classapth entry.
>
> Hm... In general, I am fine with pushing as separate changesets. It
> feels like the renaming build dirs bit belongs in this patch though (we
> are renaming the src dirs, so the build dirs with similar names should
> be updated too). But it's your call.

Pushed both hg move, extracted inner classes, adapted makefile and added headers today. For renaming 
  see below, fixing of classpaths added to todo list.
>
....
>>
>> Checked. Not necessary indedd. Although this particular one change i
>> have included into "adapted makefile" I'm not going to push it if you
>> agree with my initial recommendation  This issue is little bit spread. I
>> would like to investiagte where it is safe to remove (right now I'm sure
>> in three cases).
>
> Okay, this can be done later, then.
>
>> Anyway -  Do you think it is worthy? I can imagine few cases when
>> reproducer(testcase,extension) will would like to know something from
>> netx.jar... Although it do not need now, it can in future.. So it will
>> be work to add it.
>
> I am fairly sure testcases need netx.jar on classpath to build. As for
> test extensions...perhaps we may add jnlp or applet specific extensions
> there in the future. I am no longer sure my original suggestion was a
> good idea :)
>
>> Anyway(2:) - I would like to remove those dependencies as separate
>> changset - Right after we agree on renaming as you proposed.
>

in progress.

>
>> # HG changeset patch
>> # User Jiri Vanek <jvanek at redhat.com>
>> # Date 1340971958 -7200
>> # Node ID c913c1fd1138c833df13146da42c9d0a3512d7fd
>> # Parent  5b088a10b659082df3abe9629cc8dae9cc453dd3
>> Renamed make variables to correspond with new directory structure
>>
>> diff --git a/Makefile.am b/Makefile.am
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -12,19 +12,21 @@
>>   export TESTS_DIR=$(abs_top_builddir)/tests.build
>>
>>   export NETX_UNIT_TEST_SRCDIR=$(TESTS_SRCDIR)/netx/unit
>> -export NETX_UNIT_TEST_DIR=$(TESTS_DIR)/netx/unit
>> +export NETX_TEST_DIR=$(TESTS_DIR)/netx
>> +export NETX_UNIT_TEST_DIR=$(NETX_TEST_DIR)/unit
>>
>>   export JUNIT_RUNNER_DIR=$(TESTS_DIR)/junit-runner
>>   export JUNIT_RUNNER_SRCDIR=$(TESTS_SRCDIR)/junit-runner
>>
>>
>> -export JNLP_TESTS_ENGINE_SRCDIR=$(TESTS_SRCDIR)/test-extensions
>> -export JNLP_TESTS_ENGINE_TESTS_SRCDIR=$(TESTS_SRCDIR)/test-extensions-tests
>> -export JNLP_TESTS_SRCDIR=$(TESTS_SRCDIR)/reproducers
>> -export JNLP_TESTS_ENGINE_DIR=$(TESTS_DIR)/jnlp_testsengine
>> -export JNLP_TESTS_ENGINE_TESTS_DIR=$(TESTS_DIR)/netx/jnlp_testsengine_tests
>> -export JNLP_TESTS_SERVER_DEPLOYDIR=$(TESTS_DIR)/jnlp_test_server
>> -export JNLP_TESTS_DIR=$(TESTS_DIR)/jnlp_tests
>> +export TEST_EXTENSIONS_SRCDIR=$(TESTS_SRCDIR)/test-extensions
>> +export TEST_EXTENSIONS_TESTS_SRCDIR=$(TESTS_SRCDIR)/test-extensions-tests
>> +export REPRODUCERS_TESTS_SRCDIR=$(TESTS_SRCDIR)/reproducers
>> +export TEST_EXTENSIONS_DIR=$(TESTS_DIR)/test-extensions
>> +export TEST_EXTENSIONS_COMPATIBILITY_SYMLINK=$(TESTS_DIR)/netx/jnlp_testsengine
>> +export TEST_EXTENSIONS_TESTS_DIR=$(TESTS_DIR)/test-extensions-tests
>> +export REPRODUCERS_TESTS_SERVER_DEPLOYDIR=$(TESTS_DIR)/reproducers_test_server_deploydir
>
> I think just REPRODUCERS_DEPLOYDIR would be sufficient, but it's up to you.
>
> Looks fine to me.

It looks like you are fine also with renaming. I will push immediately after confirmation. And test 
again on my said :)

Thank you very much for cooperation on this painful refactoring!


J.

updated todo:


* investigate GPLv2 x  GPLv2 + Classpath exception for TinyHttpdImpl inspiration. I would suggest to 
email author....
* change package inside test-extensions(-tests) - net.sourceforge.jnlp "namespace"  to something 
like org.classpath.icedteaweb.tests ot  org.foo.bar ;)
* rename charReaded and  lineReaded to charRead/lineRead
* made Browsers enum members UPPERCASE
* XslowX replace by something else (imho not annotation, because then you will miss possibility to 
use it without test method, nor environment variable because you can not reset it for running 
process. Maybe some flag file?)
* EMMA filtration in process result replace by some little bit more clever middlemen
* reconsider ProcessBuilder
* investigate on "Thread.sleep(500); //this is giving to fast done proecesses's e/o readers time to 
read all. I would like to know better solution :-/ "
* fix unnecessary  NETX_DIR dependences