On Wed, 1 Jun 2022 17:34:04 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> do it as naotoj said > > test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 441: > >> 439: } >> 440: } >> 441: > > This unifies the test cases between the Set and Map factories, which > accomplishes the primary goal. Good. > > The unification is achieved through classic object-oriented polymorphism, > which works fine, though which is rather verbose. This could probably be > reduced with some tinkering on the model, but it's probably reaching the > point where additional tinkering on the model isn't worth it. I'm ok with > sticking with this approach for now. Maybe we can clean it up later, or maybe > not -- it's at least fairly straightforward. > > One issue that contributes to the verbosity is the repeated null checking. > The null checking enables the test code to proceed and end up with -1 as the > capacity if there's a null in there somewhere. This will cause the assertion > to fail. This is good in that it will call attention to itself (as opposed to > silently passing or something). However, if the test cases are set up > properly, they should never run into null. If the null checking weren't done, > an unexpected null will throw NPE, which will be caught be the framework and > reported as an error. > > That seems perfectly fine to me, so I'd suggest simply removing the null > checking. That would also reduce the bulkiness of infrastructure. @stuart-marks done. ------------- PR: https://git.openjdk.java.net/jdk/pull/8302
