RFR: 8340818: Add a new jtreg test root to test the generated documentation [v11]

[Magnus Ihse Bursie]

On Fri, 18 Oct 2024 12:21:33 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> make/RunTests.gmk line 872:
>> 
>>> 870:   $1_JTREG_BASIC_OPTIONS += -e:TEST_IMAGE_DIR=$(TEST_IMAGE_DIR)
>>> 871: 
>>> 872:   $1_JTREG_BASIC_OPTIONS += -e:DOCS_JDK_IMAGE_DIR=$$(DOCS_JDK_IMAGE_DIR)
>> 
>> I didn't include a null check here as `DOCS_JDK_IMAGE_DIR` will always hold a value (e.g `build/.../images/docs`) even if the docs don't actually exist. I will be up to the tests to skip when docs aren't found.
>
> It's fine to leave out the conditional, but the double `$$` isn't needed and I'm surprised if it actually works. That must be some lucky double evaluation somewhere in that case. 
> 
> Suggestion:
> 
>   $1_JTREG_BASIC_OPTIONS += -e:DOCS_JDK_IMAGE_DIR=$(DOCS_JDK_IMAGE_DIR)

I'm not sure why you are saying that. This is in the body of `SetupRunJtregTest`, which will be eval:ed when called, as usual. So `$$` should be used for all variable expansions. It's actually the other way around; `$` will only work due to pure "luck" in this case, and we kind of mis-used that before, splitting the set of variables into two parts, one there `$` would suffice, and one where `$$` were needed. This was just confusing, to save an extra keypress, so I've tried to use `$$` everywhere and always in these macro bodies, for consistency.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1806414120