On Thu, 17 Feb 2022 05:45:54 GMT, Stuart Marks <sma...@openjdk.org> wrote:
> It's not clear to me that test is actually testing anything useful; it's just > testing the same computation a couple different ways. Well if I don't change it, then the test will fail. > Do the changes to Class.java and the enum optimal capacity test need to be > part of this PR? They seem unrelated. But on the other hands, as they are not relative to this issue, if you think that test (`test/jdk/java/lang/Enum/ConstantDirectoryOptimalCapacity.java`)is useless and want to delete it, it shall happen elsewhere, but not in this pr. According to minimal change, I just make it can pass, but have no interest in shutting it down / heavily modify it. > 1. It should probably be a TestNG test. This will allow you to use a > DataProvider and also use TestNG assertions. good. I learned something about jtreg today and find it can invoke Junit 5. I would like to migrate this test to Junit 5, it is more familiar to me than TestNG. > 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. OK, would have a try. ------------- PR: https://git.openjdk.java.net/jdk/pull/7431
