[rfc] [icedtea-web] reproducers with custom makefile

[Adam Domurad]

I've been testing this out for a reproducer that requires all the
classes to be in their own folder. The issue I'm running into is that
$(JUNIT_JAR) is not exported (its an autoconf variable). Do you think
the required flags for compiling a testcase should be exported ? Or, as
Danesh suggested, all the required jars be locally symlinked ?

On Tue, 2012-05-29 at 18:10 +0200, Jiri Vanek wrote:
> On 05/28/2012 09:32 PM, Danesh Dadachanji wrote:
> > Hi Jiri,
> >
> > On 26/05/12 03:39 PM, Jiri Vanek wrote:
> >> Hi!
> >>
> >> We have been speaking with Danes that although current reproducers compilation and signing
> >> mechanism is very powerful, sometimes it
> >> really can not cover some extraordinary case.
> >> For this purpose we ahve come with idea to have special class of reproducers (next to signed and
> >> simple) - in this patch called custom
> >> - which will have its own makefile and will take care for itself.
> >> This is implementation of this idea. What do you think? both for idea and implementation :)
> >>
> >
> > Thank you very much for doing this! I think it will make the lives of test writers easier for those
> > unique/specific corner cases. I am definitely for it!
> >
> >> One think I do not like on this change - exporting of variables. In plain Makefile I would use
> >> "export". It looks like in Automake
> >> this is not possible. (I suspect comaptibility issues gnu x normal make), so I have exported most
> >> crucial variables manually.
> >> When I saw result Makefile I was wandering that the makefile is still working.... Well loks like
> >> was working correctly. Any hint welcomed!
> >
> > My autoconf skills are not great to say the least so hopefully someone with more experience can say
> > a few words about this. Just one question, by "I have exported most crucial variables manually", you
> > mean export so that they are available to the custom Makefile?
> Exactly. You can use the "export"ed ones as $(variable) or $$variable. Without export you can not 
> use them at all:(
> >
> >>
> >> J.
> >>
> >> * Makefile.am: Most crucial variables exported to be used by custom Makefiles
> >> (junit-jnlp-dist-custom.txt): new target scanning for directories in jnlp_tests/custom
> >> and saving them as list for future purposes.
> >> (stamps/process-custom-reproducers.stamp): new target for iterating by junit-jnlp-dist-custom.txt
> >> through jnlp_tests/custom/* and launching make prepare-reproducer in each
> >> (clean-custom-reproducers): same as above but launching make clean-reproducer
> >> (run-netx-dist-tests) now depends on stamps/process-custom-reproducers.stamp
> >> (clean-netx-dist-tests): now depends on clean-custom-reproducers
> >> * tests/jnlp_tests/README: described this mechanism a bit
> >>
> >> customMakefiles.diff
> >>
> >>
> >> diff -r 6df151bb5320 Makefile.am
> >> --- a/Makefile.am Fri May 25 11:44:13 2012 -0400
> >> +++ b/Makefile.am Sat May 26 21:05:53 2012 +0200
> >> @@ -1,58 +1,58 @@
> >> # Source directories
> >>
> >
> > [snip]
> >
> >> +export DTEST_SERVER=-Dtest.server.dir=$(JNLP_TESTS_SERVER_DEPLOYDIR)
> >> +export DJAVAWS_BUILD=-Djavaws.build.bin=$(DESTDIR)$(bindir)/$(javaws)
> >> +export DBROWSERS=-Dused.browsers=$(FIREFOX):$(CHROMIUM):$(CHROME):$(OPERA):$(MIDORI):$(EPIPHANY)
> >> +export REPRODUCERS_DPARAMETERS= $(DTEST_SERVER) $(DJAVAWS_BUILD) $(DBROWSERS)
> >> # end of `D`shortcuts
> >>
> >> +
> >
> > Extra newline?
> 
> removed.. You nitpicker :)
> 
> >
> >> # binary names
> >> javaws:= $(shell echo javaws | sed '@program_transform_name@')
> >> itweb_settings:= $(shell echo itweb-settings | sed '@program_transform_name@')
> >> @@ -492,6 +493,10 @@
> >> mkdir -p $(JNLP_TESTS_DIR)
> >> touch $@
> >>
> >> +junit-jnlp-dist-custom.txt:
> >> + cd $(JNLP_TESTS_SRCDIR)/custom/ ; \
> >> + find . -maxdepth 1 -mindepth 1 | sed "s/.\/*//"> $(abs_top_builddir)/$@
> >> +
> >
> > Are you adding this as a separate target because this target may be used outside of
> > stamps/process-custom-reproducers.stamp? If not, then can't we just merge this directly into the
> > process target? E.g. as
> >
> > + customReproducers=`find $(JNLP_TESTS_SRCDIR)/custom/ -maxdepth 1 -mindepth 1 | sed "s/.\/*//"`
> 
> Yes it is reused. I actually forgot to depend on this in clear-custom-reproducers target. Well and 
> it is good habit to generate this lists.
> 
> >
> >> junit-jnlp-dist-simple.txt:
> >> cd $(JNLP_TESTS_SRCDIR)/simple/ ; \
> >> find . -maxdepth 1 -mindepth 1 | sed "s/.\/*//"> $(abs_top_builddir)/$@
> >> @@ -648,7 +653,7 @@
> >> javaws.desktop stamps/docs.stamp launcher.build/$(itweb_settings) itweb-settings.desktop \
> >> stamps/netx.stamp stamps/junit-jnlp-dist-dirs stamps/netx-dist-tests-import-cert-to-public \
> >> stamps/netx-dist-tests-compile.stamp stamps/netx-dist-tests-compile-testcases.stamp
> >> $(JUNIT_RUNNER_JAR) \
> >> - $(TESTS_DIR)/$(REPORT_STYLES_DIRNAME) $(REPRODUCERS_CLASS_NAMES)
> >> + $(TESTS_DIR)/$(REPORT_STYLES_DIRNAME) $(REPRODUCERS_CLASS_NAMES)
> >> stamps/process-custom-reproducers.stamp
> >> cd $(JNLP_TESTS_ENGINE_DIR) ; \
> >> class_names=`cat $(REPRODUCERS_CLASS_NAMES)` ; \
> >> CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):. \
> >> @@ -662,6 +667,31 @@
> >> endif
> >> touch $@
> >>
> >> +stamps/process-custom-reproducers.stamp: stamps/junit-jnlp-dist-dirs
> >> stamps/netx-dist-tests-import-cert-to-public \
> >> + stamps/netx-dist-tests-compile.stamp stamps/netx-dist-tests-compile-testcases.stamp
> >> $(JUNIT_RUNNER_JAR) \
> >> + $(TESTS_DIR)/$(REPORT_STYLES_DIRNAME) $(REPRODUCERS_CLASS_NAMES) junit-jnlp-dist-custom.txt
> >> + . $(abs_top_srcdir)/NEW_LINE_IFS ; \
> >> + simpleReproducers=(`cat $(abs_top_builddir)/junit-jnlp-dist-custom.txt `); \
> >> + IFS="$$IFS_BACKUP" ; \
> >> + for dir in "$${simpleReproducers[@]}" ; do \
> >
> > Please rename simpleReproducers to customReproducers.
> 
> done
> 
> >
> >> + pushd $(JNLP_TESTS_SRCDIR)/custom/$$dir; \
> >> + $(MAKE) prepare-reproducer ; \
> >
> > Is there any reason for us to force devs to have a prepare-reproducer target? I feel like this is
> > something many will forget, particularly the clean-reproducer target as they may not call it ever. :/
> 
> I'm strongly for them. It will be much more buletproof and is causing nearly no effort. I'm 
> definitely  against just "make" in directory I would like to have custom targets for sure. And if no 
> then definitely forclean. They have to clean after themselves. I consoder it as benefit also for dev 
> - you will have "just make" as target for testing. And the necessary stuff will be called by make 
> prepare-reproducer. Eg deploy is not necessary for testing oon your own at all. I really would like 
> to have special targets for this! IMHO it is making live easier more then complicated.
> 
> >
> >> + popd ; \
> >> + done ;
> >> + mkdir -p stamps&& \
> >> + touch $@
> >> +
> >> +clean-custom-reproducers:
> >> + . $(abs_top_srcdir)/NEW_LINE_IFS ; \
> >> + simpleReproducers=(`cat $(abs_top_builddir)/junit-jnlp-dist-custom.txt `); \
> >> + IFS="$$IFS_BACKUP" ; \
> >> + for dir in "$${simpleReproducers[@]}" ; do \
> >> + pushd $(JNLP_TESTS_SRCDIR)/custom/$$dir; \
> >> + $(MAKE) clean-reproducer ; \
> >> + popd ; \
> >> + done ;
> >> + rm -f stamps/process-custom-reproducers.stamp
> >> +
> >
> > Rename simple to custom here too.
> 
> done
> 
> >
> >> #for global-links you must be root, for opera there do not exists user-links
> >> #although this targets will indeed create symbolic links to enable
> >> #icedtea-web plugin inside browser it is intended for testing purposes
> >> @@ -992,7 +1022,7 @@
> >> rm -rf $(TESTS_DIR)/$(REPORT_STYLES_DIRNAME)/
> >> rm -f $(TESTS_DIR)/index*.html
> >>
> >> -clean-netx-dist-tests: clean_tests_reports netx-dist-tests-remove-cert-from-public
> >> +clean-netx-dist-tests: clean_tests_reports netx-dist-tests-remove-cert-from-public
> >> clean-custom-reproducers
> >> rm -f netx-dist-tests-source-files.txt
> >> rm -rf $(JNLP_TESTS_DIR)
> >> rm -rf $(JNLP_TESTS_SERVER_DEPLOYDIR)
> >> diff -r 6df151bb5320 tests/jnlp_tests/README
> >> --- a/tests/jnlp_tests/README Fri May 25 11:44:13 2012 -0400
> >> +++ b/tests/jnlp_tests/README Sat May 26 21:05:53 2012 +0200
> >> @@ -4,7 +4,21 @@
> >> Directories are honored in srcs and in resources, but noty in testcases.
> >> Directories in signed hande their content in similar way as simle's content is handled,
> >> but in addition final jars are signed with simple testkey.
> >> -Files in advanced directory have to care about themselves, but even those can have some
> >> - parts inside simple directory, so some parts of them are processed automatically.
> >> +Files in custom directory have to care about themselves (including testcase),
> >> + but even those can have some parts inside simple directory, so some parts of them
> >> + are processed automatically.
> >> There are three reproducers – simpletest1, simpletest2 and deadlocktest, which tests
> >> test’s suite itself and serve as examples of behaviour.
> >> +If file in simple or signed directores is composed form dost, then it is placed to
> >> + directory structure eg my.dir.package will be deployed as
> >> + jnlp_test_server/my/dir/package.jar (and original kept (eg my.dir.package.jar))
> >
> > This is a little hard to understand (particularly because this concept is hard to understand :/ ).
> > What do you think of:
> >
> > If the name of a folder in simple/signed is composed of dots, then its contents are deployed from
> > under a directory structure such that each part evaluates to a folder. For example,
> > my.dir.reproducer/ will be deployed as jnlp_test_server/my/dir/reproducer.jar.
> 
> Fixed as you wish :)
> 
> >
> >> +Inside custom directory are expected directories which are handling and file or
> >> + directory structure. The only expected file is custom/reproducerName/Makefile. Upon
> >> + all custom/* are then launched make prepare-reproducer and during cleaning make
> >> + clean-reproducer Those targets are run after all simple and signed reproducers
> >> + are prepared, so they can reuse as much code as possible including eg certificates,
> >> + or testcases directories or resources or dependencies to keep this custom makefiles
> >> + as simple as possible. Some comment in makefile or readme file is recommended for
> >> + each custom reproducer to tell dependencies and what it does.
> >
> >
> > Okay, so correct me if I'm wrong here; Test writers are responsible for building and deploying
> > everything under custom/reproducer/*, namely I would have to compile, create the jar, sign it and
> > then copy everything I need into jnlp_test_server/, after which the engine will take care of it. If
> > this is correct, then this sounds more than reasonable to me!
> Exactly. With effort to reuse as much as possible from simple/signed. Eg certificates.
> >
> >> +Because of automake only small set of variables from icedtea-web Makefile is
> >> + available for custom makefiles, but feel free to export others if needed.
> >>
> >
> > I'm going to try to cp some simple reproducer and get it to mimic a deployment as if it were under
> > simple to make sure this patch works. Another review coming up! Thanks again for implementing this
> > feature!
> 
> I have tested;) So it IS working. I just did not want to spam with unnecessray reproducer as I was 
> aware you will use it soon ;)
> Changelog si the same.
> 
> Hope taht it helps.
> 
>    J.