Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ssl-tests #4835

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Add ssl-tests #4835

merged 3 commits into from
Nov 8, 2023

Conversation

zzambers
Copy link
Contributor

Adds runner for ssl-tests suite. (Code is based on CryptoTest runner with some changes and later fixes included.)

This should wait until code adding dependencies is deployed on adoptium infra, see my comment in issue for dependencies.

@smlambert smlambert requested a review from sophia-guo October 25, 2023 16:38
@llxia
Copy link
Contributor

llxia commented Oct 26, 2023

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zzambers - will await a machine to be ready so we can run some grinders before merging.

functional/security/ssl-tests/playlist.xml Outdated Show resolved Hide resolved
</then>
<else>
<if>
<matches pattern="^[1][89]" string="${JDK_VERSION}"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JDK18 and 19 are out of support. This if block should be removed.

<property name="jtregTar" value="jtreg_6_1_1"/>
</then>
<else>
<property name="jtregTar" value="jtreg_7_1_1_1"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jtreg_7_3_1_1 should be used for JDK21+.

Please check https://github.com/adoptium/aqa-tests/blob/master/openjdk/build.xml#L44-L67 for jtreg version.

@zzambers
Copy link
Contributor Author

zzambers commented Nov 2, 2023

@llxia Would it be possible to move getJtreg target and associated logic to common place from where it could be imported? Maybe putting it to TKG repo and then importing it in similar way as getDependentLibs target is being imported.

@llxia
Copy link
Contributor

llxia commented Nov 2, 2023

I agree that we should put jtreg jar logic in a commonplace. I think we should get the SSL tests PR in first, then create a separate issue for moving jtreg jar logic in a commonplace.

@smlambert
Copy link
Contributor

Created #4848 for addressing after this is delivered.

@zzambers
Copy link
Contributor Author

zzambers commented Nov 6, 2023

As getting all native clients on all systems seems to complicated. I have came up with idea, how to check for presence of native clients by jtreg harness. This can than be used to skip cases, for which dependencies are not met (without showing "fake" passes):
rh-openjdk/ssl-tests#16

This way I could hopefully enable it everywhere and adapt testing based on what is available.

@zzambers
Copy link
Contributor Author

zzambers commented Nov 7, 2023

Copy link
Contributor

@sophia-guo sophia-guo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@smlambert smlambert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @zzambers !

@smlambert
Copy link
Contributor

smlambert commented Nov 8, 2023

FYI @llxia - based on @zzambers changes (see #4835 (comment)), these can be run without worrying whether dependencies have been installed, so I will merge (and then setup a weekly run of dev.functional).

@smlambert smlambert merged commit 8270e9c into adoptium:master Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants