RFR: 8340818: Add a new jtreg test root to test the generated documentation
Erik Joelsson
erikj at openjdk.org
Tue Oct 1 13:26:42 UTC 2024
On Mon, 30 Sep 2024 18:53:57 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:
> Please review this change that adds a new test root `docs` dedicated to testing the documentation, which has been a work in progress for a while. Tests for links, encoding, HTML, accessibility will be later added in following PRs.
>
> We also define a new make target `test-docs` meant for local use and depends on the docs.
> This also adds the necessary configurations needed at Oracle.
>
> This patch includes a test `TestDocs` which serves to show developers how they are meant to resolve the docs to test them, I want to include it temporarily until better tests are added later.
>
> TIA
make/conf/jib-profiles.js line 1082:
> 1080: ],
> 1081: make_args: testOnlyMake,
> 1082: environment: {
I'm missing an environment variable communicating the location of the docs image here. Something like `DOCS_IMAGE_DIR` should be set so RunTests.gmk can pick it up and propagate it to jtreg.
make/conf/jib-profiles.js line 1095:
> 1093: if (!testedProfile.endsWith("-jcov")) {
> 1094: testOnlyProfilesPrebuiltDocs["run-test-prebuilt-docs"]["dependencies"].push(testedProfile + ".jdk_symbols");
> 1095: }
I don't think we need this. Running these tests with jcov doesn't seem to make sense to me.
make/conf/jib-profiles.js line 1113:
> 1111: profiles = concatObjects(profiles, testOnlyProfilesPrebuiltDocs);
> 1112:
> 1113: if (!new java.io.File(__DIR__, "../../README.md").exists()) {
There are several conditionals and blocks that are duplicated from the run-tests-prebuilt definitions above. I think I would prefer if you merged this into the originals instead of duplicating things. I think it's more likely that future changes will need to update all prebuilt profiles and that will be easier to get right without duplication.
test/docs/jdk/javadoc/TestDocs.java line 59:
> 57: throw new SkippedException("docs folder not found in either location");
> 58: }
> 59: }
This shouldn't be needed. We should define an environment variable that is expected to point to the docs image that RunTests.gmk sets up. Perhaps there could be java code to automatically fallback to looking in the first candidate location here, as that's the layout of the build itself. Some people prefer to bypass the makefiles and run jtreg standalone, and that would help support that usecase. I would put that java code in a common library class for the whole test root.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1782726396
PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1782723483
PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1782792422
PR Review Comment: https://git.openjdk.org/jdk/pull/21272#discussion_r1782736704
More information about the build-dev
mailing list