On Fri, 11 Mar 2022 19:40:39 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> XenoAmess has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - change KeyStructure to String >> - fix test > > Regarding the number of test cases for `tableSizeForCases` and > `populatedCapacityCases`... the issue is not necessarily with execution time > (although if the test is slow it is a problem). The issues are more around: > Is this testing anything useful, and does this make sense to the reader? > > I think we can rely on the monotonicity of these functions. If populating a > map both with 49 and with 96 mappings results in a table length of 128, we > don't need to test that all the intermediate inputs also result in a table > length of 128. Including all the intermediate inputs makes the source code > more bulky and requires future readers/maintainers to hunt around in the long > list of tests to figure out which ones are significant. Really, the only ones > that are significant are the boundary cases, so just keep those. Adding more > tests that aren't relevant actually does hurt, even if they execute quickly. > So: please cut out all the extra test cases that aren't near the boundary > cases. > > I'm not sure what's going on with the build. The builds are in GitHub Actions > and they aren't necessarily reliable, so I wouldn't worry about them too > much. I'll run the final version through our internal build/test system > before integration. (I've also done this previously, and the results were > fine.) @stuart-marks all the changes to the test you requested at last review comments are done. please have a look, thanks! ------------- PR: https://git.openjdk.java.net/jdk/pull/7431
