[8u] RFR: 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

[Andrew Hughes]

On 21/04/2020 11:06, Severin Gehwolf wrote:
> Hi Andrew,
> 
> On Tue, 2020-02-18 at 12:25 +0000, Andrew Hughes wrote:
>>
>> On 29/11/2019 16:42, Severin Gehwolf wrote:
>>> On Fri, 2019-11-29 at 16:09 +0000, Hohensee, Paul wrote:
>>>> Looks good.
>>>
>>> Thanks for the review, Paul!
>>>
>>>> Separately, how difficult would it be to backport the SA test infra? Should we consider doing it?
>>>
>>> I haven't looked at it in detail, but it may be something we should
>>> consider doing. I doubt it's difficult, but it'll be a chunk of work.
>>>
>>> It basically amounts to looking at
>>> test/hotspot/jtreg/serviceability/sa/ and checking what functionality
>>> exists in 8u then porting it over. Not sure how much of the other test
>>> support libraries would need to get backported too.
>>>
>>> Thanks,
>>> Severin
>>>
>>>> Thanks,
>>>> Paul
>>>>
>>
>> Could we not include the tests but ProblemList them for now? I'm very
>> wary of just dropping tests on the floor, because it means someone else
>> needs to find and restore them under a separate bug later. You probably
>> need to knock the 'jtreg' out of the path so it fits into the current
>> OpenJDK 8u setup.
> 
> In JDK 8u there is only one ProblemList.txt (in jdk repo). So there is
> no easy way to problem list a test out of the box. I could remove the
> @test annotation so that it doesn't run. Is such an approach going to
> help anybody, though? It'll just not run and would need another bug to
> get enabled anyway. Of course I can do that if you insist (like in
> [1]), but it opens up the door for it to bit-rot.
> 
> Personally, it's either biting the bullet and backporting the entire SA
> testing infra (and this test). Or not having this test at all. For
> reference (and adhoc testing), there would be the JDK 11 test to fall
> back on. I'm strongly for the latter in this case.
> 
>> JDK-8081037: "serviceability/sa/ tests time out on Windows" (introduces
>> LingeredApp.java) and JDK-8190198: "SA: Framework for writing 'jhsdb
>> clhsdb' commands tests and testcases for some of the commands" seem to
>> be the key issues here, but there are probably others.
> 
> Right. Do we really need to bring in two more bugs for this small fix
> in SA code, though? There is a balance to strike. It's hard enough to
> get fixes in. Even without backporting 2 other dependencies for tests.
> Getting 8u test code into a state where 11u is will require some
> significant amount of work across the board :-/
> 
> Thanks,
> Severin
> 
> [1] http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196969/jdk8/02/webrev/
> 

I think it's preferable to have it there, even disabled, than expecting
someone to know about it on some future date when the dependent tests
are added. It's also easier to fix it up by adding one line back than it
is to dig out the original changeset.

Worst case, we can always delete it later.

Approved and pushed v02, will be part of jdk8u262-b04.
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222