Hi Roger, On Wed, Oct 24, 2018 at 5:23 PM Roger Riggs <roger.ri...@oracle.com> wrote: > > Hi Thomas, > > Thanks for adding the test. > > There's a feature of jtreg that was exposed a couple of month ago that > can be used to deal with running the test too many times. > > There can be multiple @test blocks with different @requires. > > * @run main/othervm/timeout=300 Basic * @run main/othervm/timeout=300 > -Djdk.lang.Process.launchMechanism=fork Basic * * @test * @requires > (os.family == "linux") * @run main/othervm/timeout=300 > -Djdk.lang.Process.launchMechanism=posix_spawn Basic > >
Does not seem to work, sorry: 24 /* 25 * @test 26 * @bug 4199068 4738465 4937983 4930681 4926230 4931433 4932663 4986689 27 * 5026830 5023243 5070673 4052517 4811767 6192449 6397034 6413313 28 * 6464154 6523983 6206031 4960438 6631352 6631966 6850957 6850958 29 * 4947220 7018606 7034570 4244896 5049299 8003488 8054494 8058464 30 * 8067796 31 * @key intermittent 32 * @summary Basic tests for Process and Environment Variable code 33 * @modules java.base/java.lang:open 34 * @run main/othervm/timeout=300 Basic 35 * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=fork Basic 36 * @test 37 * @requires (os.family == "liinux") 38 * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=posix_spawn Basic 39 * @author Martin Buchholz 40 */ If I have @requires (os.family == "linux") all three variants are executed - ok. Then I wanted to have a negative test, so I misnamed linux as "liinux" and would have expected the first @test block to fire, the second to be ignored. But in fact: thomas@mainframe /shared/projects/openjdk/jtreg $ ../jtreg-prebuilt/jtreg/bin/jtreg -jdk ../jdk-jdk/output-slowdebug/images/jdk/ ../jdk-jdk/source/test/jdk/java/lang/ProcessBuilder/Basic.java Test results: no tests selected So to me it looks like as if the @requires tag is valid for both @test blocks, not only the one it appears in. > I'll run this through our build system too. > > To fully test out using posix_spawn in many more different scenarios the > build system should be augmented to be able to use it as the default for > an entire build/CI/CD/test runs. Sure! But I think this is out of scope for this patch. Thanks, Thomas > > Thanks, Roger > > On 10/24/2018 10:35 AM, Thomas Stüfe wrote: > > Hi all, > > > > version 2 of Davids patch, with test changes: > > > > http://cr.openjdk.java.net/~stuefe/webrevs/JDK-8212828-posix_spawn.patch/webrev.01/webrev/ > > > > Executed the test on my local Ubuntu box, works fine. Submit job runs. > > > > About the test: I added a new line: > > > > * @run main/othervm/timeout=300 Basic > > * @run main/othervm/timeout=300 -Djdk.lang.Process.launchMechanism=fork > > Basic > > + * @run main/othervm/timeout=300 > > -Djdk.lang.Process.launchMechanism=posix_spawn Basic > > > > The way I understand those tests work is that the first line causes > > the test to be run with the default launch mechanism, the second line > > with jdk.lang.Process.launchMechanism=fork explicitly. The third line, > > added by me, will now explicitly test with posix_spawn. I did not > > limit the platforms, since posix_spawn should now be available on all > > platforms, so it should work on all platforms. But then, on all other > > platforms it has already been the default. > > > > This is still a bit iffy: On Windows, jdk.lang.Process.launchMechanism > > gets ignored, so we just executed the same test twice (now three > > times)? And now we execute the posix_spawn variant twice on all > > platforms where this is the default, so line 1 and 3 are the same? You > > see I am not a jtreg expert :) Can I specify a @run directive for only > > one platform? In that case I would limit the explicit posix_spawn test > > to Linux. > > > > Note however that if we really abondon vfork in the future and make > > posix_spawn the default, this test becomes simpler too. > > > > Best, Thomas > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Oct 24, 2018 at 3:36 PM David Lloyd <david.ll...@redhat.com> wrote: > >> On Wed, Oct 24, 2018 at 1:05 AM Thomas Stüfe <thomas.stu...@gmail.com> > >> wrote: > >>> Review: > >>> > >>> - copyright dates need updating on the C-sources > >>> > >>> - I opt for "#if defined(__solaris__) || defined(_ALLBSD_SOURCE) || > >>> defined(_AIX) || defined(__linux__)" to be removed completely from > >>> unix-specific source files. The ifdef now covers all OpenJDK Unix > >>> platforms. > >> Here's a version with these changes. > >> > >> -- > >> - DML >
