On Tue, 30 Aug 2022 19:27:26 GMT, Stuart Marks <sma...@openjdk.org> wrote:

>> Raffaello Giulietti has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8065554: MatchResult should provide values of named-capturing groups
>
> test/jdk/java/util/regex/NamedGroupsTests.java line 45:
> 
>> 43: 
>> 44:         testMatchResultStartEndGroup();
>> 45:     }
> 
> I haven't gone through all the tests in great detail (yet), but it occurs to 
> me that we potentially have THREE implementations of some of the logic, and 
> strictly speaking we should test all the code paths. The three 
> implementations are in:
> 
> 1. Matcher
> 2. Matcher$ImmutableMatchResult
> 3. MatchResult's default methods
> 
> I took a quick look and it looks like Matcher and 
> Matcher$ImmutableMatchResult override the default methods, so the default 
> methods themselves need to be tested. This is essentially testing the 
> `@implSpec`. The typical way to do that is to have the test create its own 
> MatchResult implementation(s). There might need to be implementations that do 
> and do not implement `namedGroups`, in order to test UOE. They might also 
> need some state to cover various cases of no-match, has-match with and 
> without group names, etc.

An implementation that overrides `namedGroups` without overriding the other 
methods accepting group names is `Matcher$ImmutableMatchResult`, which is 
already exercised in the tests.

> (Hm, might want to take another look at the specs regarding this issue.)

Not sure who wants to take another look. If that it's you, then I'll wait with 
the CSR.
I'll change the method names to something a bit more speaking.

-------------

PR: https://git.openjdk.org/jdk/pull/10000

Reply via email to