Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 GMT, Leo Korinth wrote: >> One can select a testcase by ID when running a jtreg test case directly from >> jtreg (using the testcase.java#testID syntax). However, this has not been >> possible to do when launching jtreg indirectly from make. >> >> This fix attempts to address this issue. I have not tested this thoroughly >> yet, I wanted to show the code to get feedback first. The idea is to *not* >> emulated destructive imperative assignments through the use of eval. I >> instead try to handle it in a functional style without reassigning >> variables. I have tried to make the change as small as possible. >> >> I am not used to change the build system, so I would appreciate thorough >> review. >> >> `IfAppend` and `IfPrepend` are general purpose functions. If similar >> functions exists elsewhere inside the code base or in make itself I should >> probably use those instead. If they do not exist, they might be useful >> elsewhere and maybe should be placed in a common make file instead of the >> RunTests.gmk file. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > wip I will add a comment: # The test selector starting with a hash (#testid) will be stripped by all evals # and will be reinserted in the last line by calling TestID (if it is present). Arguably I should also remove the hash part explicitly, would you like that? I think it might be more clear what the code does, but on the other hand it would maybe signal that it is a parsing strategy which it is not. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 GMT, Leo Korinth wrote: >> One can select a testcase by ID when running a jtreg test case directly from >> jtreg (using the testcase.java#testID syntax). However, this has not been >> possible to do when launching jtreg indirectly from make. >> >> This fix attempts to address this issue. I have not tested this thoroughly >> yet, I wanted to show the code to get feedback first. The idea is to *not* >> emulated destructive imperative assignments through the use of eval. I >> instead try to handle it in a functional style without reassigning >> variables. I have tried to make the change as small as possible. >> >> I am not used to change the build system, so I would appreciate thorough >> review. >> >> `IfAppend` and `IfPrepend` are general purpose functions. If similar >> functions exists elsewhere inside the code base or in make itself I should >> probably use those instead. If they do not exist, they might be useful >> elsewhere and maybe should be placed in a common make file instead of the >> RunTests.gmk file. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > wip I think this looks good, but Magnus should still take a look. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Mon, 6 Jun 2022 07:55:09 GMT, Erik Joelsson wrote: >> Leo Korinth has updated the pull request incrementally with one additional >> commit since the last revision: >> >> wip > > make/RunTests.gmk line 444: > >> 442: $(strip $(foreach parser,ParseCustomTestSelection >> ParseGtestTestSelection ParseMicroTestSelection ParseJtregTestSelection >> ParseSpecialTestSelection \ >> 443: ,$(call $(parser),$1))) >> 444: endef > > As this is logically a one-liner macro, please format according to > guidelines. Also try to keep lines shortish. Something like this: > > Suggestion: > > ParseTestSelection = \ > $(strip $(foreach parser, ParseCustomTestSelection ParseGtestTestSelection > ParseMicroTestSelection \ > ParseJtregTestSelection ParseSpecialTestSelection, \ > $(call $(parser), $1) \ > )) I made it a one-liner, and I refactored the code to use `or` - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 GMT, Leo Korinth wrote: >> One can select a testcase by ID when running a jtreg test case directly from >> jtreg (using the testcase.java#testID syntax). However, this has not been >> possible to do when launching jtreg indirectly from make. >> >> This fix attempts to address this issue. I have not tested this thoroughly >> yet, I wanted to show the code to get feedback first. The idea is to *not* >> emulated destructive imperative assignments through the use of eval. I >> instead try to handle it in a functional style without reassigning >> variables. I have tried to make the change as small as possible. >> >> I am not used to change the build system, so I would appreciate thorough >> review. >> >> `IfAppend` and `IfPrepend` are general purpose functions. If similar >> functions exists elsewhere inside the code base or in make itself I should >> probably use those instead. If they do not exist, they might be useful >> elsewhere and maybe should be placed in a common make file instead of the >> RunTests.gmk file. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > wip @magicus could you also take a look please? - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
> One can select a testcase by ID when running a jtreg test case directly from > jtreg (using the testcase.java#testID syntax). However, this has not been > possible to do when launching jtreg indirectly from make. > > This fix attempts to address this issue. I have not tested this thoroughly > yet, I wanted to show the code to get feedback first. The idea is to *not* > emulated destructive imperative assignments through the use of eval. I > instead try to handle it in a functional style without reassigning variables. > I have tried to make the change as small as possible. > > I am not used to change the build system, so I would appreciate thorough > review. > > `IfAppend` and `IfPrepend` are general purpose functions. If similar > functions exists elsewhere inside the code base or in make itself I should > probably use those instead. If they do not exist, they might be useful > elsewhere and maybe should be placed in a common make file instead of the > RunTests.gmk file. Leo Korinth has updated the pull request incrementally with one additional commit since the last revision: wip - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/704158c8..26a62eb0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=9028&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=9028&range=01-02 Stats: 14 lines in 2 files changed: 0 ins; 4 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
