RFR: 8219585: [TESTBUG] sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java passes trivially when it shouldn't

[Severin Gehwolf]

Hi Daniel,

On Tue, 2019-02-26 at 16:50 +0000, Daniel Fuchs wrote:
> Hi Severin,
> 
> On 25/02/2019 15:31, Severin Gehwolf wrote:
> > Hi Daniel,
> > 
> > Thanks for the review!
> > 
> > Latest webrev:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8219585/02/webrev/
> > 
> [...]
> > I'm not sure we need a larger timeout. It's 20 seconds on my machine
> > which seems more than enough. What makes you think we'd need to get
> > this increased and it would actually do anything for test stability?
> 
> Meanwhile I launched your proposed changes through our
> test system. The good news is that there was no failure
> (even when repeating the test 25 times), and without
> changing the 5 timeout - so that may be OK.
> Maybe my machine has something that slows it down.
> Or Maybe I should have used a timeout factor of 4.
> It does pass if I use -timeout:4 when invoking it with jtreg.

FWIW, I use 'make test TEST="..."'. Isn't the testing system using that
too?

> The not so good news is that it seems none of our test
> machine has a non-loopback address configured for local host.
> So even with your changes, the test always trivially pass.
> 
> I see the message says "Ignoring manual test" - but the
> test is not manual?

Like you've determined, it's manual. In the sense that it needs
specific config to /etc/hosts on some test machine as described in the
comment of the test. It needs to know some, non-loopback, IP address it
can bind to for the purpose of the test. Then the test will use that
address + loopback. That's the minimal test setup. Pre-patch, even such
a setup was not sufficient. That's what this patch fixes.

When I was working on that patch years ago, I think one testing machine
got configured in a way so that it wouldn't trivially pass. That may
have changed at some point in time?

> [...]
> 
> > Yes. I've tried to avoid those issues by allowing one non-loopback
> > address and 127.0.0.1 explicitly. Is that a reasonable assumption?
> > 
> 
> WRT using the loopback as an alternate configuration,
> I wonder - should we add the loopback address before
> checking size() < 1?
> Not sure how stable the test will be in that case.

It wouldn't help anything. In that case it wouldn't be a regression
test anymore as it would pass *prior* JDK-6425769 *and* after. With a
non-loopback address added too, it would fail prior and pass after.

> Also should we use InetAddress.getLoopbackAddress() instead
> of using "127.0.0.1" directly?

InetAddress.getLoopbackAddress() is less deterministic than using IPv4
address. Interestingly the javadoc mentions it may return any loopback
address, not just 127.0.0.1 or ::1. Not sure whether that's a problem
or not, I'd be fine to change that if you prefer.

Thanks,
Severin