[rfc][icedtea-web] improvements to reproducers engine

[Danesh Dadachanji]

On 23/05/12 07:17 AM, Jiri Vanek wrote:
> On 05/22/2012 08:58 PM, Danesh Dadachanji wrote:
>> Hi Jiri,
>>
>> On 20/05/12 10:18 AM, Jiri Vanek wrote:
>>> Hi!
>>>
>>> During last week I was asked by Danesh and Saad to introduce more hierarchical server and
>>> whitelist for reproducers.
>>> The following patches should add those features. Can you guys (requesters:) mind to review if they
>>> match yours needs?
>>> During the writing i found one bug in current implementation - redundant quotes. I fixed this in
>>> third changeset.
>>> Zipped attachment is example of source and result.
>>>
>>
>> Thank you very much for doing this! Greatly appreciated!
>
> hhmmhmh. I'm still little bit unsure about usefulness (probably only because of lack of ideas (except sub dirs (and so lost pacakges in
> jars))... So I'm glad to hear that appreciated it ;)
>

Definitely, I plan to use it as soon as it goes in!

[snip]

>>>
>>> *smallEnchancemtnsToreproducers-whiteList.diff*
>>>
>>
>> Any thoughts on reproducers that depend on other ones' jars? For example, the spaces test and the
>> info/title/vendor tests use simpletest1.jar to run. If simpletest1 is not whitelisted then the
>> engine will throw failures when running them. Are we expecting testers to know which jars depend on
>> which? If this "dependency" is not going to be checked by the Makefile, then we should definitely
>> add a note to the wiki page stating the various dependencies.
>
> I had this on mind when I wrote it. Whitelist just filter runs, not compilation and preparation. All reproducers (simple, signed and in
> future also special) are compiled (,signed) , deployed , resources copied, testcases compiled. But to the final list for runinning are
> included just those who match the regexes in list
>
> I would really like to avoid some descriptions or checks in reproducers(dependences)! They can make the engine to heavyweight.


Oh my apologies for the noise, I did not realize this was just a runtime whitelist. Awesome then I am perfectly happy with the change. 
Feel free to push!

[snip]

> smallEnchancemtnsToreproducers-jarsInDirs2.diff
>
>
> diff -r c74bf4aa138c Makefile.am
> --- a/Makefile.am	Wed May 23 10:42:45 2012 +0200
> +++ b/Makefile.am	Wed May 23 13:06:33 2012 +0200
> @@ -544,8 +544,40 @@
>    	mkdir -p stamps&&  \
>    	touch $@
>
> +stamps/change-dots-to-paths.stamp: stamps/netx-dist-tests-sign-some-reproducers.stamp
> +	pushd  $(JNLP_TESTS_SERVER_DEPLOYDIR); \
> +	types=(simple signed); \
> +	for which in "$${types[@]}" ; do \
> +	  . $(abs_top_srcdir)/NEW_LINE_IFS ; \
> +	  simpleReproducers=(`cat $(abs_top_builddir)/junit-jnlp-dist-$$which.txt `); \
> +	  IFS="$$IFS_BACKUP" ; \
> +	  for dir in "$${simpleReproducers[@]}" ; do \
> +	    if test "$${dir:0:1}" = "." ; then \
> +	      echo "reproducer $$dir starts with dot. It is forbidden" ; \
> +	      exit 5; \
> +	    fi; \
> +	    if test "$${dir:(-1)}" = "." ; then \
> +	      echo "reproducer $$dir ends with dot. It is forbidden" ; \
> +	      exit 5; \
> +	    fi; \

> +	    q=`expr index "$$dir" .`; \
> +	    r=$$? ; \
> +	    if [ $$r = 0  ]; then \

I don't think this if-clause is needed anymore, basename/dirname are smart enough to account for this. Let's say dir="MyAwesomeTest", 
then path="." and file="MyAwesomeTest". Feel free to keep it there as a sanity check but I don't think it's necessary. No need to 
repost a patch if you just remove the if (unless other issues arise).

> +	      slashed_dir="./$${dir//.//}" ; \
> +	      path="`dirname $$slashed_dir`" ; \
> +	      file="`basename $$slashed_dir`.jar" ; \
> +	      echo "copying $$dir.jar to $$path as $$file" ; \
> +	      mkdir --parents $$path ; \
> +	      cp $$dir".jar" "$$path"/"$$file" ; \
> +	    fi ; \
> +	  done ; \
> +	done ; \
> +	popd ; \
> +	mkdir -p stamps&&  \
> +	touch $@
> +
>   #this always tries to remove  previous testcert
> -$(EXPORTED_TEST_CERT): stamps/netx-dist-tests-sign-some-reproducers.stamp netx-dist-tests-remove-cert-from-public
> +$(EXPORTED_TEST_CERT): stamps/change-dots-to-paths.stamp netx-dist-tests-remove-cert-from-public
>   	keytool -export -alias $(TEST_CERT_ALIAS) -file $(EXPORTED_TEST_CERT) -storepass $(PRIVATE_KEYSTORE_PASS) -keystore $(PRIVATE_KEYSTORE_NAME)
>
>   stamps/netx-dist-tests-import-cert-to-public: $(EXPORTED_TEST_CERT)
> @@ -955,6 +987,7 @@
>   	rm -f stamps/netx-dist-tests-prepare-reproducers.stamp
>   	rm -f stamps/netx-dist-tests-compile-testcases.stamp
>   	rm -f stamps/netx-dist-tests-sign-some-reproducers.stamp
> +	rm -f stamps/change-dots-to-paths.stamp
>   	rm -f junit-jnlp-dist-simple.txt
>   	rm -f junit-jnlp-dist-signed.txt
>   	rm -f $(REPRODUCERS_CLASS_NAMES)
>

Great stuff, thanks again for doing all of this! Feel free to push to HEAD

Cheers,
Danesh