GEODE-7167: eliminate NPE when parsing locators property

[Bill]

GEODE-7167

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

• Has your PR been rebased against the latest commit within the target branch (typically develop)?

• Is your initial contribution a single, squashed commit?

• Does gradlew build run cleanly?

• Have you written or updated unit tests to verify your changes?

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

[dschneider-pivotal]

This looks good. The only suggestion I have is you might want to try to improve the coverage of you unit test. I saw the parse code looking for '@' but I didn't see that covered and some of the other error conditions (like port being 0) didn't look covered. Try running your new unit test in your IDE with coverage and see how well the new code is covered. To get some of it covered you might need to use a spy and extract some of the code (like new InetSocketAddress) into a method that you can then mock to return a mocked InetSocketAddress that will say it is not loopback (so that you can generate the error about attempting to join...). It looks to me like you have already improved the unit test coverage of this code.
Did one of your new tests fail with an NPE before your fixes?

[Bill]

Yes @dschneider-pivotal, before the product fix, unresolveableAddress() failed since an NPE was thrown.

I did consider adding tests for parsing of the "host" part and opted not too. Your comment has made me reconsider…

[lgtm-com]

This pull request introduces 1 alert when merging 8fed26e into 5f70160 - view on LGTM.com

new alerts:

• 1 for Useless comparison test

[onichols-pivotal]

will this fix GEODE-6153 as well?

[Bill]

Yes @onichols-pivotal. Those are dupe tickets.

[mhansonp]

@Bill Should I merge this?