On Fri, 3 Mar 2023 18:04:11 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8302590: Add String.indexOf(int ch, int fromIndex, int toIndex) > > The latest version looks good. Thanks for going through the process of > getting this to a good place. One minor comment is that the test should > probably in the java/lang/String rather than the CompactString sub-directory. > @AlanBateman There's a `IndexOf.java` test file already in the > `CompactString` folder, which is why the new test file is located there, > side-by-side to the current one. Both classes extend class `CompactString`, > also residing in that folder, to get access to `String` constants defined > there. > > I have no problem in moving the new test file to the parent folder, but would > like to understand more about the distinction between the two groups of > tests. Could you expand a bit on this to help me getting a better picture? TIA String dates from JDK 1.0 and didn't historically have many tests except for some regression tests that accumulated along the way. Recent work on new String APIs (repeat, indent, ..) added to the tests in java/lang/String. The CompactString sub-directory is from the JEP 254 work in JDK 9 - it needed good tests to exercise all methods/cases. It might get onto someone radar sometime to do some consolidation and make it less confusing as to whether to put new tests. ------------- PR: https://git.openjdk.org/jdk/pull/12600
