I am a big fan of precedent and clarity, but that being said, using ${EXE_SUFFIX} is a bit more clunky at each use site in each shell test.  But, I guess a test that uses the suffix a lot could define EXE=${EXE_SUFFIX} and use the shorter form.

My scripts currently show the following counts for shell tests:

test/langtools: 6 ... this is believable, even though it is higher than expected
test/jdk: 243 ... this seems credible
test/hotspot/jtreg: 15 ... this number seems way low; I may need to figure a better static analysis for this repo.

We would need to get buy-in from the various component teams before doing any mass-update to shell tests.

-- Jon


On 1/16/19 10:52 AM, Andrew Luo wrote:
Sounds good to me.  I had been a bit busy recently so hadn't yet had a chance 
to respond.

I do recommend naming the variable EXE_SUFFIX though, since that's what we call 
it in the build scripts.  Although if you do strongly prefer EXE instead, it 
doesn't matter to me that much.  Obviously this approach will require 
modification to many of the tests.  I can probably handle a good portion of the 
tier 1 test cases - less than ~100 test cases are failing (on my system that is 
- not all of which are shell scripts anyways).  But I don't know how many shell 
scripts we have in total.  Perhaps if we find some common patterns we can just 
use a regex to find/replace what needs to be changed...

Thanks,

-Andrew

-----Original Message-----
From: Jonathan Gibbons <[email protected]>
Sent: Wednesday, January 16, 2019 10:46 AM
To: Erik Joelsson <[email protected]>; Andrew Luo 
<[email protected]>; [email protected]
Subject: Re: [PATCH] Fixes for running tests on WSL



On 01/11/2019 02:24 PM, Erik Joelsson wrote:
On 2019-01-11 14:12, Jonathan Gibbons wrote:
3 options that come to mind are ...

1. jtreg sets new env variables for any/all JDK tools that may be
invoked, including java and javac, and the scripts are updated to use
the new variables

2. jtreg sets a single new env variable EXE, which is either empty or
".exe". A bit ugly for anyone not used to using WSL

3. all the magic is somehow done in the tests, with no additional
change to jtreg

I think 1 or 2 is preferable to 3. There will be a lot of duplication
of this logic in the tests otherwise.
I think 2 is preferable to 1, since it is only a single env variable instead of 
many. I'll update jtreg to set EXE in WSL mode.  Then, we just have to figure 
what to do with the tests.

-- Jon


I also note the orthogonal option, which is to continue the migration
away from using shell tests as much as possible.

I certainly agree that we need to move away from shell tests long term!

/Erik

-- Jon


On 01/11/2019 01:45 PM, Erik Joelsson wrote:
With the new jtreg, the test is launched correctly. It's now a
problem with the test. When WSL launches a Windows executable, it
needs to run it with the full name (including .exe). This particular
test tries to run things like:

/mnt/d/erik/jdk-jib-wsl/build/windows-x64/images/jdk/bin/javac
-J-Xmx768m -J-XX:MaxRAMPercentage=1 -J-ea -J-esa pkg/A.java
pkg/B.java

Which fails because it needs to be javac.exe.

To fix these tests, one thing we need to do is find a good way of
identifying that the JDK under test is of Windows type and in that
case add .exe to the executables. Note that checking if the
execution environment happens to be WSL is not good enough. The
relevant part in this case is identifying the JDK under test.

/Erik

On 2019-01-11 10:14, Jonathan Gibbons wrote:
I'm pretty sure I know the problem. I'll post an update to jtreg.

-- Jon

On 01/11/2019 09:44 AM, Erik Joelsson wrote:
Hello,

With the new jtreg and the patch, normal java tests seem to work
fine.

I tried some simple shell tests and they fail. I took a test in
langtools as an example:

open/test/langtools/tools/javac/Paths/Class-Path.sh

I invoked it like this:

$ make test TEST=langtools/tools/javac/Paths/Class-Path.sh

It fails with the following:

--------------------------------------------------
TEST: tools/javac/Paths/Class-Path.sh TEST JDK:
d:\erik\jdk-jib-wsl\build\windows-x64\images\jdk

ACTION: shell -- Failed. Execution failed: exit code 127
REASON: User specified action: run shell Class-Path.sh
TIME:   0.125 seconds
messages:
command: shell Class-Path.sh
reason: User specified action: run shell Class-Path.sh elapsed
time (seconds): 0.125
STDOUT:
STDERR:
sh: 0: Can't open
/mnt/d:/erik/jdk-jib-wsl/open/test/langtools/tools/javac/Paths/Cla
ss-Path.sh

It seems the translation of the path fails. Further down in the
jtr file, this looks to be the command line Jtreg is trying to run:

     wsl.exe \\
         sh
'/mnt/d:\\/erik/jdk-jib-wsl/open/test/langtools/tools/javac/Paths/Class-Path.sh'

It should be noted that WSL does not understand windows paths like
Cygwin does. By default it mounts every Windows drive x in /mnt/x,
but this is configurable so should not be assumed. There is a
tool, wslpath, similar to cygpath, which can be used to translate.
It's also possible to translate environment variables by adding
them to WSLENV.

Perhaps using WSLENV would be a reasonable trick here. When
running wsl.exe, also set something like this in the env:

JTREG_TEST_PATH=d:/erik/jdk-jib-wsl/open/test/langtools/tools/java
c/Paths/Class-Path.sh

WSLENV=$WSLENV:JTREG_TEST_PATH/p

then rewrite the command as:

$ wsl.exe sh $JTREG_TEST_PATH

/Erik

On 2019-01-11 08:51, Jonathan Gibbons wrote:
Erik,

Can you try a test run with the latest jtreg bits, and post the
results for me to look at, to see why tests are failing and what,
if anything, needs to be done?

-- Jon

On 1/10/19 9:02 PM, Andrew Luo wrote:
I've updated my patch per the latest jtreg changes.  Tested this
with the latest code synced from the jtreg repository, seems
like shell tests do get run (but don't pass, tests will need to
be changed).

Thanks,

-Andrew

-----Original Message-----
From: Jonathan Gibbons <[email protected]>
Sent: Wednesday, January 9, 2019 1:39 PM
To: Erik Joelsson <[email protected]>; Andrew Luo
<[email protected]>; [email protected]
Subject: Re: [PATCH] Fixes for running tests on WSL

Erik,

I have pushed a version of Andrew's patch for jtreg into the
jtreg repo.

With regard to Andrew's proposed patch for make/RunTests.gmk,
the name of the option to use for jtreg is now just "-wsl'.

-- Jon

On 01/07/2019 01:10 AM, Erik Joelsson wrote:
Hello Andrew,

If the patch gets accepted to Jtreg, this looks good. We will
need to wait for Jtreg support before pushing this though.

/Erik

On 2019-01-06 20:34, Andrew Luo wrote:
Hi Everyone,

I've gotten shell tests to run on WSL with some changes to
jtreg and a small change to the OpenJDK gmake files.  Most of
them are still not passing (I believe one or two of them did
just work out of the box after these changes as failures +
error count dropped; previous errors + previous failures <
current failures; also "errors"
dropped
to zero), as the scripts themselves will need to be changed,
however, at least now they can actually run...  My patch with
proposed changes are attached.

I've sent the corresponding jtreg changes to the
code-tools-dev mailing list:
https://mail.openjdk.java.net/pipermail/code-tools-dev/2019-Ja
nuary/0

00464.html

Let me know if you have any feedback/comments.

Thanks,

-Andrew


Reply via email to