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

Erik Joelsson erikj at openjdk.org
Fri Oct 18 12:28:09 UTC 2024


On Fri, 18 Oct 2024 09:14:06 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

>> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   use existing DOCS_JDK_IMAGE_DIR to be consistent.
>>   
>>   no ifneq check before assigning the env variable because  DOCS_JDK_IMAGE_DIR is set even docs do not exist. It will be up to the test to check if the files are there
>
> 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)

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

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


More information about the build-dev mailing list