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

[Omair Majid]

Hi Jiri,

On 06/28/2012 07:55 AM, Jiri Vanek wrote:
> Attached new set of patches. Should be final imho.

Please see comments in-line below.

> Resume of future tasks:
> 
> * 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 :-/ "
> 
> You:
> * study TestInBrowser changeset;)
>  - the "toExec" you have mentioned is imho already solved  - browsers
> proxies have string argument with by-autoconf set value of full path to
> executable.
> * possible help with slee(500) and with ProcessBUilder if you will have
> lack of duties;)

Thanks for the nice summary!

> # HG changeset patch
> # User Jiri Vanek <jvanek at redhat.com>
> # Date 1340880424 -7200
> # Node ID aea1cafcefbfab4e69995854ffcecd78a674d751
> # Parent  31f78e1476a0af93ace536580fa4f54b37e5f2e7
> Makefile.am  adapted to recent (three changesets) refactoring.
> 
> diff --git a/Makefile.am b/Makefile.am
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -17,9 +17,12 @@
>  export JUNIT_RUNNER_DIR=$(TESTS_DIR)/junit-runner
>  export JUNIT_RUNNER_SRCDIR=$(TESTS_SRCDIR)/junit-runner
>  
> -export JNLP_TESTS_ENGINE_SRCDIR=$(TESTS_SRCDIR)/netx/jnlp_testsengine
> -export JNLP_TESTS_SRCDIR=$(TESTS_SRCDIR)/jnlp_tests
> -export JNLP_TESTS_ENGINE_DIR=$(TESTS_DIR)/netx/jnlp_testsengine
> +
> +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 PRIVATE_KEYSTORE_NAME=teststore.ks

Could you rename the variable so they are more accurate? Something like
JNLP_TESTS_ENGINE_SRCDIR -> TEST_EXTENSIONS_SRCDIR makes more sense to
me. Also, can you update the build dir names to match the src dir names?

> @@ -634,8 +637,24 @@
>  	mkdir -p stamps && \
>  	touch $@
>  
> +netx-dist-tests-tests-source-files.txt:

s/netx-dist-tests-tests-source-files/test-extensions-sources-files/

> +	find $(JNLP_TESTS_ENGINE_TESTS_SRCDIR) -name '*.java' | sort > $@
> +
> +stamps/netx-dist-tests-tests-compile.stamp: stamps/netx.stamp \

A clearer name for this stamp too, please.

> + stamps/junit-jnlp-dist-dirs netx-dist-tests-tests-source-files.txt \
> + stamps/netx-dist-tests-compile.stamp
> +	mkdir -p $(JNLP_TESTS_ENGINE_TESTS_DIR);
> +	$(BOOT_DIR)/bin/javac $(IT_JAVACFLAGS) \
> +	 -d $(JNLP_TESTS_ENGINE_TESTS_DIR) \
> +	 -classpath $(JUNIT_JAR):$(NETX_DIR)/lib/classes.jar:$(JNLP_TESTS_ENGINE_DIR) \

The tests for the test-extensions depend on netx to build? Seems rather
weird.

> +	 @netx-dist-tests-tests-source-files.txt && \
> +	mkdir -p stamps && \
> +	touch $@

I dont think any of these ";" or "&& \" are needed. It should be fine if
each command was executed in a new shell.

Everything else looks fine for now.

Cheers,
Omair