On Thu, 5 Nov 2020 12:46:08 GMT, Conor Cleary <ccle...@openjdk.org> wrote:
> The primary goal of this fix was to refactor > NetworkInterface/UniqueMacAddressesTest.java to make use of the > jdk.test.lib.NetworkConfiguration class instead of java.net.NetworkInterface. > This was due to the original failure being related to the use of the 'awdl' > interface on macOS, which should be skipped for functional tests. The > NetworkConfiguration class accounts for this when returning the list of > present interfaces (originally retrieved using > NetworkInterface.getNetworkInterfaces()) as well as for other OS specific > properties. The effects of these changes can be seen on L59 and L101-106. > > In addition to these changes, some modifications were made to the test in the > interest of keeping it concise and readable but were made bearing in mind the > original logging of the test was already up to a good standard (and I sought > to keep it that way). To summarize the other changes: > > - When using NetworkConfiguration::interfaces to read in a stream of > NetworkInterface objects, the objects are collected into a list of records > (see [JEP 395](https://openjdk.java.net/jeps/395)) to avoid multiple calls to > NetworkInterface::getHardwareAddress (which can throw a SocketException) > throughout the test. > - When checking if the MAC addresses are unique, the testMacAddressesEqual() > method is not called unless the interface names are different. If > testMacAddressesEqual() returns true, the calling method will simply return > instead of updating the value of a boolean variable. > - Majority of logging was moved to the testMacAddressesEqualMethod() so that > only logs for comparisons are carried out. There is scope for additional > logging of, for example, the list of present interfaces returned by > createNetworkInterfaceList() if deemed necessary by review. Changes requested by dfuchs (Reviewer). test/jdk/java/net/NetworkInterface/UniqueMacAddressesTest.java line 38: > 36: * @test > 37: * @bug 8021372 > 38: * @summary Tests that the MAC addresses returned by > NetworkConfiguration.probe() are unique for each adapter. I would keep the summary as in the original test. We are really testing the NetworkInterfaces returned by `NetworkInterface.getNetworkInterfaces` - even though we are skipping some of them that we know are problematic. test/jdk/java/net/NetworkInterface/UniqueMacAddressesTest.java line 108: > 106: } > 107: > 108: record NetIfPair(String interfaceName, byte[] address) {} Nit: I'd move that to line 46. It would avoid having to wonder what `NetIfPair` is until you reach the end of the file. ------------- PR: https://git.openjdk.java.net/jdk/pull/1076
