On Thu, 30 Oct 2025 22:53:59 GMT, Ioi Lam <[email protected]> wrote:
> In the previous fix (https://github.com/openjdk/jdk/pull/28035), I added
> `OutputAnalyzer::match(String regexp)`, but it didn't work because by default
> regular expressions do not match across newlines.
>
> I fixed this by re-working `OutputAnalyzer::match()`, etc, to use
> `Pattern.MULTILINE`.
>
> I tried rerunning the test on macos 26 but couldn't reproduce the condition
> in the bug report. However, I added sanity test in this version
> (https://github.com/openjdk/jdk/commit/e690e97262575d083017635fa837ab267686bfe9)
> and the new regexp seems to catch the output and correctly come to this part
> of the test case:
>
>
> if (forceBase >= end) {
> throw new SkippedException("Failed to force ccs to any of the given
> bases. Skipping test.");
> }
Changes requested by stefank (Reviewer).
test/lib/jdk/test/lib/process/OutputAnalyzer.java line 359:
> 357: private boolean matchesHelper(String s, Pattern pattern) {
> 358: return s != null && pattern.matcher(s).find();
> 359: }
This function is named `matchesX` so why is it using `Matcher::find` instead of
`Matcher::matches`? That seems confusing.
test/lib/jdk/test/lib/process/OutputAnalyzer.java line 364:
> 362: Pattern pattern = Pattern.compile(regexp, Pattern.MULTILINE);
> 363: return matchesHelper(stdout, pattern) || matchesHelper(stderr,
> pattern);
> 364: }
I'd like to suggest that you don't hard-code `stdout` and `stderr` here and use
something like this (with updated name as suggested above):
Suggestion:
private boolean findIn(String regexp, String... strings) {
Pattern pattern = Pattern.compile(regexp, Pattern.MULTILINE);
for (String string : strings) {
if (pattern.matcher(string).find()) {
return true;
}
}
return false;
}
and then the code you have that looks like this:
return matchesHelper(getStdout(), null, regexp);
can get rid of the `null`:
return findIn(regexp, getStdout());
-------------
PR Review: https://git.openjdk.org/jdk/pull/28077#pullrequestreview-3402888668
PR Review Comment: https://git.openjdk.org/jdk/pull/28077#discussion_r2480391654
PR Review Comment: https://git.openjdk.org/jdk/pull/28077#discussion_r2480406005
