Openjdk-tests: Ensure proper test coverage for SSL/TLS implementation

Created on 15 Sep 2020  路  14Comments  路  Source: AdoptOpenJDK/openjdk-tests

We almost shipped a JDK 15 with a broken SSL/TLS implementation on all platforms (bug report). This was caused by https://github.com/AdoptOpenJDK/openjdk-build/pull/2033. I cannot find failing tests that are related to that problem in TRSS. But that may be due to my inexperience with TRSS.

We missed similar problems in the past but on a much smaller scale: https://github.com/AdoptOpenJDK/openjdk-support/issues/33. So it seems there's something amiss with out testing coverage.

Example of a broken nightly build: https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk11u/job/jdk11u-linux-x64-hotspot/716/

enhancement

All 14 comments

extended.jdk target was running weekly but has not been enabled in nightlies (due to lack of triage resources) - it would have some of the tests that would catch this issue as it includes jdk_security1, jdk_security2, jdk_security3, jdk_security4, jdk_security_infra make targets.

see https://github.com/AdoptOpenJDK/openjdk-tests/issues/1523 (less fun tasks for 2020 include triaging all extended.openjdk tests) and https://github.com/AdoptOpenJDK/openjdk-tests/issues/1522 (proposed additions to AQA v2)

Marked this as top priority, essentially re-enable extended.openjdk target and include it in the release pipeline. If we do not have the resources to run it nightly, it can be run weekly but must be included in the release runs.

https://github.com/AdoptOpenJDK/openjdk-build/pull/2024 sets up the ability to define tests as nightly and weekly from the build pipelines, and notes that a release build should include both sets of tests.

@aahlenst - see https://github.com/AdoptOpenJDK/openjdk-tests/compare/master...smlambert:secTests which takes the testcases from your branch, adds build/playlist/testng files so that the testcases will be run as part of our suite and produce junit style output. New test target called SecurityTests is currently added into extended.functional.

Some sample Grinders:
jdk8 hotspot xlinux: Grinder/4599
jdk11 hotspot xlinux: Grinder/4600
jdk15 openj9 xlinux: Grinder/4601
jdk8 hotspot aarch64: Grinder/4602

Awesome, thanks.

Is there a good way for "automatic" version selection, for example "Run only on 11 and only if it's 11.0.9 or newer"?

If its at a test target level (all test cases in a group that is defined in a playlist), then a mechanism is the <subset> tag:

<test>
        <testCaseName>SecurityTests</testCaseName>
        <command>$(JAVA_COMMAND) $(JVM_OPTIONS) -cp $(Q)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)SecurityTests.jar$(Q) org.testng.TestNG $(Q)$(TEST_RESROOT)$(D)testng.xml$(Q) -d $(REPORTDIR) -testnames SecurityTests -groups $(TEST_GROUP) -excludegroups $(DEFAULT_EXCLUDE); \
        $(TEST_STATUS)</command>
        <levels>
            <level>extended</level>
        </levels>
        <groups>
            <group>functional</group>
        </groups>
        <subsets>
            <subset>11</subset>
        </subsets>
    </test>

The subset tag is currently only at the granularity of major version (and can support special branches like Valhalla/Panama). We do not currently have a mechanism to say 'only 11 and not less than 11.0.9', though may be useful to move that logic into a shared location for use by all tests. We had not yet encountered a need to match on update versions.

For checking inside of functional test code, we lay down the functional test material from openj9, so the VersionCheck class is available, but again, not serving update version info presently.

<subsets>
    <subset>8</subset>
    <subset>11</subset>
</subset>

Is that legal syntax to express to run on 8 and 11 only? Is there some syntax to say 11 or newer?

... though may be useful to move that logic into a shared location for use by all tests. We had not yet encountered a need to match on update versions.

Do you think about a Java class or something to extend the XML declaration? We have plenty of use cases: JFR is only available on 8u262 or newer, TLS 1.3 only on 8u272 and newer, Shenandoah only on 11.0.9 and newer.

re: https://github.com/AdoptOpenJDK/openjdk-tests/issues/1963#issuecomment-724790298 that is correct syntax if you only want to run against 8 and 11.

<subsets>
    <subset>11+</subset>
</subset>

for version 11 and up (so since we are currently running jdk15 tests, then in practical terms, would show up in jdk11 and jdk15 pipelines... though if someone tried to run a jdk11-jdk16 pipelines, tests would be run in that range).

We were discussing an update to how we represent subsets / versions in playlist xml files, tagging @renfeiw who would have good insight into evolving what we can do there (and also because he is currently extending the playlist xml files to handle more granular exclusion capabilities).

re https://github.com/AdoptOpenJDK/openjdk-tests/issues/1963#issuecomment-724808372 We will update the tag "subset" to "version" later on to be more explicit. We are also thinking of using Interval notation such as [11, 11], [11.0.9, infinite) to represent java version more precisely.

thanks @renfeiw

@aahlenst - we can do this in stages, get your testing in place using PRE_223_PATTERN temporarily (with the plan to move the handling of more precise java version info to TKG in a second stage, need to create an issue so its not lost/forgotten)... or what is your preference?

I'll start with what we have and continue from there. Or even easier: We can just merge your branch Shelley if you convert it into a PR. Beforehand, it might make sense to run a grinder with a nightly from Sep. 13 or 14 to check that the tests fail.

jdk11 hotspot Grinder (on Sept 13th nightly) indeed shows the tests fail / catch the issue

Screen Shot 2020-11-10 at 8 00 04 PM

Hooray! So after merging the PR, the last step is to enable nightly runs of those tests.

Because they are added into extended.functional, they will automatically be run nightly, since I recently added that top-level target to the nightly test list.

Was this page helpful?
0 / 5 - 0 ratings