Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v5]
On Tue, 7 Jun 2022 18:15:30 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: > > use test id in comments I apologize for the late review. The latest version look very good indeed, arguably even much better than the original. (Apart from it fixing the bug, that is.) Thank you for contributing to the build system! You're welcome back any time you want. ;-) - Marked as reviewed by ihse (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 [v5]
On Tue, 7 Jun 2022 18:15:30 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: > > use test id in comments 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 [v5]
> 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: use test id in comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/ac9631e2..a6d51c2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9028=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9028=03-04 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 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