On Thu, 17 Feb 2022 05:45:54 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix test > > OK, good progress. Yes, leaving ConcurrentHashMap to another PR is fine. > > Do the changes to Class.java and the enum optimal capacity test need to be > part of this PR? They seem unrelated. It's not clear to me that test is > actually testing anything useful; it's just testing the same computation a > couple different ways. > > The test seems reasonable enough and is a good start. There are a number of > things that could be improved about it though. > > 1) It should probably be a TestNG test. This will allow you to use a > DataProvider and also use TestNG assertions. > > 2) There are 12 test cases here, which seems amenable to using a > DataProvider. You could try to make a little "combo" test that combines the > three classes with the four ways of creating them, but it might be difficult > to do that without using reflection. It might be easier to write a table with > 12 cases, even if there is a bit of repetition. > > 3) Instead of writing reflective code to create the maps, it's probably > easier to use suppliers that create the maps into the desired state. Each of > the 12 test cases should have a lambda that does the creation. The test > method then invokes the supplier and makes its assertions over the resulting > map instance. > > 4) The `fill12` method can be used in an expression if it returns its > argument. > > 5) Create a map with 12 entries as part of the test setup, and then just use > it as the copy constructor argument. > > Note, I'll be on vacation next week, so there will be a gap in my responses > during that time. @stuart-marks please have a look again, thanks! ------------- PR: https://git.openjdk.java.net/jdk/pull/7431
