RFR: 7903659 Verify Problemlist contents [v6]

[Jonathan Gibbons]

On Tue, 12 Mar 2024 11:19:48 GMT, Ludvig Janiuk <lujaniuk at openjdk.org> wrote:

>> This change introduces the flag `--verify-exclude` (short form `-i`) which will run additional checks on the problemlists passes through `-exclude`. If any of the checks fails, jtreg prints a helpful message and refuses to start running tests.
>
> Ludvig Janiuk has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - oops
>  - fix strange bash contruct

src/share/classes/com/sun/javatest/regtest/tool/ExcludeFileVerifier.java line 53:

> 51:     public boolean verify(File file, List<String> validTestNames) {
> 52:         ArrayList<String> usedTestNames = new ArrayList<String>();
> 53:         ArrayList<Check> checks = new ArrayList<Check>();

If old school, it would be more appropriate to use just `List` on the LHS, if that is sufficient.

If new school, use `var`.

Either way, you don't need duplicate type parameters. Either use diamond syntax or `ver`.

src/share/classes/com/sun/javatest/regtest/tool/ExcludeFileVerifier.java line 56:

> 54:         checks.add(new LineFormatCheck());
> 55:         checks.add(new TestExistsCheck(validTestNames));
> 56:         checks.add(new DuplicateCheck(usedTestNames));

I like the style of a potentially extensible set of checks ;-)

src/share/classes/com/sun/javatest/regtest/tool/ExcludeFileVerifier.java line 93:

> 91:         if (line.equals("")) return true;
> 92:         if (line.charAt(0) == '#') return true;
> 93:         return false;

Could be simplified to 

return line.isBlank() || line.trim().startsWith("#");

src/share/classes/com/sun/javatest/regtest/tool/Tool.java line 1447:

> 1445: 
> 1446:         if (hadErrors) {
> 1447:             error("Cannot run because an exclude list had errors, printed above. Either resolve them or remove the exlude list.");

typo: exlude

src/share/classes/com/sun/javatest/regtest/tool/i18n.properties line 262:

> 260: help.main.nativepath.desc=Path to location of native libraries and programs needed \
> 261:     by the tests.
> 262: help.main.ve.desc=Verify contents of exclude files

"validate" may be a better word than "verify"

test/verifyexclude/VerifyExcludeTest.gmk line 190:

> 188: 	$(BUILDTESTDIR)/verifyexclude.exist.ok \
> 189:     $(BUILDTESTDIR)/verifyexclude.match.format.1.ok \
> 190:     $(BUILDTESTDIR)/verifyexclude.shortform.ok

Something may have gone wrong with the indentation here

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

PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541614857
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541617577
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541622077
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541628852
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541634696
PR Review Comment: https://git.openjdk.org/jtreg/pull/181#discussion_r1541642271