[icedtea-web] reviewer needed - added engine for lunching reproducers

Fri May 20 03:28:05 PDT 2011

On 05/17/2011 05:45 PM, Omair Majid wrote:
> On 05/17/2011 09:31 AM, Jiri Vanek wrote:
>>>> diff -r 2b1a69f4c54b Makefile.am
>>>> --- a/Makefile.am Tue May 10 11:16:17 2011 -0400
>>>> +++ b/Makefile.am Wed May 11 13:52:03 2011 +0200
>>>> @@ -12,11 +12,17 @@
>>>> NETX_UNIT_TEST_SRCDIR=$(TESTS_SRCDIR)/netx/unit
>>>> TESTS_STYLES_SRCDIR=$(TESTS_SRCDIR)/$(REPORT_STYLES_DIRNAME)
>>>> JUNIT_RUNNER_SRCDIR=$(TESTS_SRCDIR)/junit-runner
>>>> +NETX_DIST_TEST_SRCDIR=$(TESTS_SRCDIR)/netx/dist
>>>> +NETX_JNLP_TEST_SRCDIR=$(TESTS_SRCDIR)/jnlp_tests
>>>>
>>>> TESTS_DIR=$(abs_top_builddir)/tests.build
>>>> NETX_UNIT_TEST_DIR=$(TESTS_DIR)/netx/unit
>>>> TESTS_STYLES_DIR=$(TESTS_DIR)/$(REPORT_STYLES_DIRNAME)
>>>> JUNIT_RUNNER_DIR=$(TESTS_DIR)/junit-runner
>>>> +NETX_DIST_TEST_DIR=$(TESTS_DIR)/netx/dist
>>>> +NETX_TESTCASES_DIR=$(TESTS_DIR)/netx/dist_testcases
>>>> +TESTS_SERVER_DIR=$(TESTS_DIR)/test_server
>>>> +TESTS_JNLP_DIR=$(TESTS_DIR)/jnlp_tests
>>>>
>>>
>>> I would suggest renaming these directories (and variables). I am not
>>> sure what 'dist' really signifies. The jnlp tests should probably in
>>> tests/netx/jnlp/ --they are for netx only and not for the plugin.
>> You yourself suggested 'make check-dist' - so I took this word to
>> signify its belongings.
>
> Well, 'make distcheck' is a commonly known target. In a source tree though, the meaning of tests/netx/dist/ is not clear.
>
>> ok - tests/netx/jnlp/ will be more suitable
>>
>> NETX_JNLP_TEST_SRCDIR contains sources of reproducers applications,
>> NETX_DIST_TEST_SRCDIR contains helping classes and virtual server sources.
>> NETX_DIST_TEST_DIR contains compiled helping classes, server etc.
>
> If you can come up with something more descriptive than DIST, then please use that. Otherwise DIST should be fine for now.
>
>> NETX_DIST_TESTCASES_DIR have been removed.
>> TESTS_SERVER_DIR - contains reproducers jars, resources... And is root
>> directory for server. TESTS_JNLP_DIR contains compiled reproducers
>>
>> So since teh testcases_dir is removed, i think the names are ok.
>>
>> Or you can vote for:
>> NETX_JNLP_TEST_SRCDIR -> JNLP_REPRODUCERS_SRCDIR
>> NETX_DIST_TEST_SRCDIR - > JNLP_TESTS_HELPINGCLASSES_SRCDIR
>> NETX_DIST_TEST_DIR -> JNLP_TESTS_HELPINGCLASSES_BUILDDIR
>> TESTS_JNLP_DIR -> JNLP_REPRODUCERS_BUILDDIR
>> TESTS_SERVER_DIR -> JNLP_TESTS_SERVER_DIR
>>
>
> I thought we were using the convention that *_SRCDIR indicates a source diretory and *_DIR indicates a build directory. Otherwise I think they are alright. I dont care too much what variables are called; I care much more about the source tree paths: /
>
renamed, hoe that good enough!

>>>>
>>>> JUNIT_RUNNER_JAR=$(abs_top_builddir)/junit-runner.jar
>> ...snip...
>>>> +
>>>> CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):.
>>>> \
>>>> + $(BOOT_DIR)/bin/java -Dtest.server.dir=$(TESTS_SERVER_DIR)
>>>> -Djavaws.build.bin=$(DESTDIR)$(bindir)/javaws \
>>>> + -Xbootclasspath:$(RUNTIME) CommandLine $$class_names \
>>>
>>> I just realized (sometimes I can be slow ;) ) that if you are passing
>>> all these java properties, you can also pass along the values of
>>> -Dicedtea-web.bin.name and -Dicedtea-web.bin.path and avoid having to
>>> run these tests only after 'make install'.
>>
>> Hmm.. Do not understand this very much. Is there something useful to run
>> this tests without all-local? Where comes values of this variables from?
>> Where are they from? From where will they come when no make all-local done?
>>
>
> Sorry if I wasn't clear: you have to build (all-local) to test of course. But I was suggesting that if you pass along all the right variables, that you could (possibly, have to double-check) run the just-built javaws binary. You might be able to skip having to install it to check. But it's not really that big a deal for now. You can ignore it :)
>

ignored:)

>>>> + > stdout.log 2> stderr.log ; \
>>>> + cat stdout.log ; \
>>>> + cat stderr.log>&2
>>>> +
>>>> netx-unit-tests-source-files.txt:
>>>> find $(NETX_UNIT_TEST_SRCDIR) -name '*.java' | sort> $@
>>>>
>>>> @@ -496,7 +564,7 @@
>>>> cat stdout.log ; \
>>>> cat stderr.log>&2
>>>>
>>>> -clean-netx-tests: clean-netx-unit-tests clean-junit-runner
>>>> +clean-netx-tests: clean-netx-unit-tests clean-junit-runner
>>>> clean-netx-dist-tests
>>>> if [ -e $(TESTS_DIR)/netx ]; then \
>>>> rmdir $(TESTS_DIR)/netx ; \
>>>> fi
>>>> @@ -513,6 +581,16 @@
>>>> rm -rf $(NETX_UNIT_TEST_DIR)
>>>> rm -f stamps/netx-unit-tests-compile.stamp
>>>>
>>>> +clean-netx-dist-tests:
>>>> + rm -f netx-dist-tests-source-files.txt
>>>> + rm -rf $(TESTS_JNLP_DIR)
>>>> + rm -rf $(TESTS_SERVER_DIR)
>>>> + rm -rf $(NETX_DIST_TEST_DIR)
>>>> + rm -f stamps/netx-dist-tests-compile.stamp
>>>> + rm -f stamps/netx-dist-tests-prepare-reproducers.stamp
>>>> + rm -f stamps/netx-dist-tests-compile-testcases.stamp
>>>> + rm -f junit-jnlp-dist-dirs.txt
>>>> +
>>>> # plugin tests
>>>
>>> I cant see a way to run these tests using a standard 'make' target.
>>
>> But they are running this way now (or NOT????? -confused-)
>> Is there any issue I can't see?
>>
>
> They are? Mea culpa then. As I said elsewhere in the email, I did not try and build this patch.
no change here
>
>>>
>>>>
>>>> if ENABLE_PLUGIN
>>>> diff -r 2b1a69f4c54b tests/jnlp_tests/README
>>>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>>>> +++ b/tests/jnlp_tests/README Wed May 11 13:52:03 2011 +0200
>>>> @@ -0,0 +1,1 @@
>>>> +Each file in simple must follows naming convention and is
>>>> compiled/jared automaticky into server's working directory and
>>>> content of resources likewise. The name of jnlp is independent, ant
>>>> there can be even more jnlps for each future jar.
>>>> diff -r 2b1a69f4c54b
>>>> tests/jnlp_tests/simple/simpletest1/resources/simpletest1.jnlp
>>>
>>> Please fix the spelling error(s?).
>>
>> Done..
>>
>> Each file in simple must follows naming convention and is compiled/jared
>> automatically into server's working directory and content of resources
>> likewise. The name of jnlp is independent, ant there can be even more
>> jnlps for each future jar.
>>
>
> More typos ;)

Enlarged (because of ˇˇ) and hope that fixed :)
>
>>>
>>> Why the '/simple/' in the path?
>>
...anip...
>> codebase="[@]address[@]" , which will then be changed in java tests or
>> the port will be handled outside junit-run and so it will be sedded..
>> somwehow.
>>
>
> If it works, no need to make life more complicated :)
>
>>>
>>>> +<information>
>>>> +<title>simpletest1</title>
>>>> +<vendor>NetX</vendor>
>>>> +<homepage href="http://jnlp.sourceforge.net/netx/"/>
..snip..
>>> this trivial test. But just to give an idea. And thanks for adding a
>>> concrete example to show how actual tests would be like.
>>
>> Licence - yes!
>> Those trivial test are included exactly and mainly as proof of concept.
>> But I was thinking about commitng them - as guide others and as warning
>> that something goes very very bad (with java, javaws or testsuite or
>> with whatever) when even those two fails.
>>
>
> That's actually a good idea. Can you please add a note in the README? (you probably want to do that as a separate patch).
>
done
>>>
>>>> +package simpletest1;
>>>> +
>>>> +public class SimpleTest1{
>>>> +
>>>> + public static void main(String[] args){
>>>> + System.out.println("Good simple javaws exapmle");
>>>> + }
>>>> +}
>>>
>>> Is there an advantage to placing this class in a separate package?
>>
>> Sure! I believe that classes should never be in "default" (== empty)
>> package. In this case it also helps me to jar them clearly.
>
> Can you elaborate on why default package is a bad idea for test cases?
>
>>

as agreed. Now it is simple/name/srcs|resources|testcases/whatever (resources copied recursively, rest compiled by *)
>>>
>>> I dont see any other issues, but I will build and test out this patch
>>> as soon as I get some extra time.
>> Thank you very much for review and hints.
>
> No, thank YOU for creating the patch and testing it and bearing with my very nit-picky reviews!
>
> Cheers,
> Omair
new patch attached, and except following gissue I consider it as final. You can test it as make run-netx-dist-tests after apply.

but -  as discussed yesterday - process assasin is working BAD with javaws. The behaviour is quite curious and I'm afradid it can be hidden bug (feature?) of javaws. The javaws process is never finished (no retur value from it) But its streams ARE closed (or at least -1 char received).
So the way how I'm getting std out/err, however strange- works best. Process assasin ca not work - it kill javaws(or somethig?), lost its stdout/err but is still haging something else. I'm ashamed for this drawback and I will investigate this in future. For now I think it must be enough.

I have added deadlock reproducer test, which can help to track this issue.

regards J.

2011-05-20 Jiri Vanek <jvanek at redhat.com>
	* tests/jnlp_tests: directory for reproducers
	* tests/jnlp_tests/advanced: reproducers which must care about
	 deploying and compiling thmselves
	* tests/jnlp_tests/simple: reproducers compiled, jared and deployed
	automatically
	* tests/jnlp_tests/simple/name/srcs|testcases|resources/: sourcefiles,
	resources and testaces for simple reproducers
	* tests/jnlp_tests/simple/deadlocktest: test for tracing not-killable
	javaws
	* tests/jnlp_tests/simple/simpletest1: tutorial test
	* tests/jnlp_tests/simple/simpletest2: tutorial test with exception
	* tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ResourcesTest.java:
	tests for server basic functionality
	* tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java:
	implementation of server to produce jnlps and resources. Implementation
	of helpers to run javaws process.
	Makefile.am: new variables pointing to structure above;
	(junit-jnlp-dist-dirs.txt): prepare destination directory structure
	(stamps/netx-dist-tests-prepare-reproducers.stamp):compile tescascases of simple reproducers
	(netx-dist-tests-source-files.txt): lookup for server and helping classes
	(stamps/netx-dist-tests-compile.stamp): compile server and helping classes
	(stamps/netx-dist-tests-compile-testcases.stamp): compile, jar and deploy all simple testcases and their resources
	(run-netx-dist-tests): after make install run junit testsuite upon reproducers on virtual server
	(clean-netx-tests): added dependence on  clean-netx-dist-tests
	(clean-netx-dist-tests): deleting  of reproducers

-------------- next part --------------
A non-text attachment was scrubbed...
Name: j-reproducers_engine-new.patch
Type: text/x-patch
Size: 47669 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110520/34802049/j-reproducers_engine-new.patch 