RFR: 7903659 Verify Problemlist contents [v2]

[Ludvig Janiuk]

On Wed, 6 Mar 2024 23:31:09 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> Ludvig Janiuk has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - copyright, more
>>  - copyright
>>  - Pattern.compile
>>  - formatting, pass explicit PrintWriter
>>  - Return empty array, not null
>
> src/share/classes/com/sun/javatest/regtest/tool/Tool.java line 646:
> 
>> 644:         },
>> 645: 
>> 646:         new Option(NONE, MAIN, null, "-ve", "-verifyexclude") {
> 
> I might have gone for `--verify-exclude` but this is OK

I also find `--verify-exclude` more pleasing, but for reasons unknown to me, all the existing jtreg flags use a single dash even for the long versions. The majority also seem to concatenate words with no separator, although there are examples of both kebab-case and camelCase. For these reasons, I would opt for the flag as it is written.

> test/excludeverify/ExcludeVerifyTest.gmk line 45:
> 
>> 43: 		|| (exit 0)
>> 44: 	$(GREP) -s "Must follow:" $(@:%.ok=%/log 2>&1)
>> 45: 	$(GREP) -s "The fully qualified test must exists." $(@:%.ok=%/log 2>&1) && exit 1 || exit 0
> 
> Here and on similar lines, the appearance of `2>&1` _inside_ the `$(@...)` expression seems weird to the point of wrong.  I don't know what you're trying to do or if any redirection is actually necessary here. If you copied this code from elsewhere, I'd be interested to know where from.

I'm pretty sure I copied from https://github.com/openjdk/jtreg/blob/master/test/exclude/ExcludeTest.gmk, the same pattern is found on https://github.com/openjdk/jtreg/blob/master/test/exclude/ExcludeTest.gmk#L44.

> test/excludeverify/ExcludeVerifyTest.gmk line 149:
> 
>> 147: 	$(BUILDTESTDIR)/excludeverify.good.ok \
>> 148: 	$(BUILDTESTDIR)/excludeverify.id.ok \
>> 149: 	$(BUILDTESTDIR)/excludeverify.exist.ok
> 
> Overall general comment for the file:
> Well done; you did good!

Thank you.

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1517923691
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1517907400
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1517916832