RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v2]

[Leo Korinth]

On Mon, 6 Jun 2022 06:50:44 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   wip
>
> make/RunTests.gmk line 39:
> 
>> 37: ################################################################################
>> 38: 
>> 39: HASH:=\#
> 
> HASH is already defined in MakeBase.gmk so should be available here.

thanks, in the new version I do not even use it.

> make/RunTests.gmk line 47:
> 
>> 45: define IfPrepend
>> 46: $(if $(strip $1),$(strip $2)$(strip $1),)
>> 47: endef
> 
> These two probably fits better in make/common/Utils.gmk. 
> 
> Also please have a look at the [Code Conventions for the Build System](http://openjdk.java.net/groups/build/doc/code-conventions.html). One liner macros like this we like to format like this:
> 
> 
> IfPrepend = \
>   $(if $(strip $1),$(strip $2)$(strip $1),)

I moved them both to `make/common/Utils.gmk`.

> make/RunTests.gmk line 51:
> 
>> 49: define TestID
>> 50: $(subst .java,,$(subst .java$(HASH),,$(suffix $(notdir $1))))
>> 51: endef
> 
> This macro deserves a comment explaining what it's trying to do.

I added a description and made it a one-liner as well

> make/RunTests.gmk line 53:
> 
>> 51: endef
>> 52: 
>> 53: define HashTestID
> 
> This one too.

I removed HashTestID and added the functionality directly into TestID

> make/RunTests.gmk line 377:
> 
>> 375: # either absolute or relative to any of the TEST_BASEDIRS or test roots.
>> 376: define ParseJtregTestSelection
>> 377: $(call IfAppend, \
> 
> Please adjust indentation of the whole body here.

Fixed

> make/RunTests.gmk line 448:
> 
>> 446: # Now intelligently convert the test selection given by the user in TEST
>> 447: # into a list of fully qualified test descriptors of the tests to run.
>> 448: TESTS_TO_RUN:=$(strip $(foreach test,$(TEST),$(call ParseTestSelection,$(test))))
> 
> I think this rewrite is more elegant, but logically there is a difference as it no longer breaks early. I would like to have Magnus' opinion on this as he wrote this originally.
> 
> Style wise, please leave spaces around operators and space after comma when possible. The top level strip should handle any whitespace issues caused by this.

I changed the code to use `$(or)` instead of `$(foreach)`, and I added spaces.

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

PR: https://git.openjdk.java.net/jdk/pull/9028