On Fri, 18 Aug 2023 01:49:20 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Ben Perez has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Changed function spec wording, updated test > > test/jdk/java/security/Provider/ChangeProviders.java line 84: > >> 82: } >> 83: >> 84: //Ensure that providers inserted at position 0 or > n are placed > > nit: this sentence is a bit hard to parse, maybe it's easier to just state > that positions out of the [1...n] range is placed at the position n+1? Actually, you don't need another provider impl, you could have just removed Foo and then proceed with the Foo provider with your insertProviderAt(...) testing. With the current test, when you start testing, the provider count is n+1, which can be confusing mixed with your comment of using n as the current provider count... Thus, I'd suggest you just remove the Foo provider and re-use it. This way, there is less code and all is consistent. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14925#discussion_r1297907238
