RFR (S) 8223146 [TESTBUG] new test vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/ fails on Windows

[David Holmes]

Hi Jc,

On 1/05/2019 12:36 pm, Jean Christophe Beyler wrote:
> Hi David & Serguei,
> 
> @Serguei: I know from experience that submit-repo builds on various 
> platforms but I'm never sure what tests are actually run. Since this is 
> a testbug for windows, I'm just wondering if we could/should confirm it 
> fixed that case; I'm fairly confident since my tests with the right 
> options show the same exact failure on linux before the patch but still...
> 
> @David: Thanks for the review, the inclusion path is this actually
> 
>   test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h ->
>   /usr/include/inttypes.h ->
> /usr/lib/gcc/x86_64-linux-gnu/7/include/stdint.h ->
>   /usr/include/stdint.h
> 
> I've added an explicit inclusion to the c++ file in my local branch, 
> seemed cleaner :)

Okay - thanks!

> The submit repo passed so if you could check if it did run on windows, 
> that would be great; or I can just submit and push and we can see, 
> whichever you prefer.

AFAICS these tests are not run in jdk-submit. We saw the failure in 
tier5 testing. But I used your jdk-submit build to run this test on 
Windows and it passed. So please go ahead and push the fix.

jib > --------------------------------------------------
jib > TEST: 
vmTestbase/nsk/share/ExceptionCheckingJniEnv/exceptionjni001/TestDescription.java
jib >   build: 1.282 seconds
jib >   compile: 1.266 seconds
jib >   driver: 0.593 seconds
jib >   build: 0.11 seconds
jib >   compile: 0.11 seconds
jib >   main: 0.203 seconds
jib > TEST RESULT: Passed. Execution successful
jib > --------------------------------------------------

I also just noticed this test is located in nsk/share/ - why is that? 
AFAIK "share" is for utility classes not for actual tests. ??

Thanks,
David
-----

> Thanks both for the review!
> Jc
> 
> On Tue, Apr 30, 2019 at 5:54 PM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     Hi Jc,
> 
>     On 1/05/2019 10:00 am, Jean Christophe Beyler wrote:
>      > Hi all,
>      >
>      > (second day in a row is a "charm"; my apologies for the double post)
>      >
>      > I figured out the problem for this test bug, I put additional
>     data in
>      > the bug itself for tracking.
> 
>     Yep - never use "long" as its size will vary. :)
> 
>      > Webrev: http://cr.openjdk.java.net/~jcbeyler/8223146/webrev.00/
>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8223146
> 
>     Change looks okay to me. Only query is where the INT32_MIN
>     definition is
>     coming from? I see it in stdint.h which isn't included.
> 
>      > Note: If this solution is ok to all, I'll drop the problem list
>     webrev
>      > and push this one; if this lingers and I get LGTMs for the
>     problem list,
>      > I'll push that one and then we can work out this problem/solution.
> 
>     Yep that's a good plan. Better to fix the bug than ProblemList.
> 
>      > (I've tested this as much as I could and have pushed this on the
>      > submit-repo but I'm unsure that would test this test on Windows; if
>      > someone could test it, that would be perhaps a good idea as well).
> 
>     I can't test on Windows either. I'll check the jdk-submit job to see
>     that it does run the test on Windows as expected.
> 
>     Thanks,
>     David
> 
>      > Thanks for your help,
>      > Jc
> 
> 
> 
> -- 
> 
> Thanks,
> Jc