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

Make smoke tester script work on main branch (java 17) [LUCENE-10463] #11499

Closed
asfimport opened this issue Mar 12, 2022 · 6 comments
Closed

Comments

@asfimport
Copy link

The smoke tester script has been obsoleted on main after upgrading to Java 17. To enable nightly smoke tests on Jenkins for main, its target java version should be bumped to 17.

In addition to bump the java version, it looks it should be refactored not to hard-code target java version. I feel it'd be better to make it coordinate with the Gradle distribution task.


Migrated from LUCENE-10463 by Tomoko Uchida (@mocobeta), resolved Mar 15 2022
Pull requests: #748

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Just to clarify, there are two "java version" - the target java version of the artifacts (this is fixed when build) and the runtime java version the script runs on (this can be changed by command-line arguments).

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Would you share your thoughts on that, @dweiss  - does it make sense to add a Gradle task that just outputs (to stdout or a temporary file) the target java version (sourceCompatibility and targetCompatibility) to verify the jar Manifests in the smoke tester? Or, in a more straightforward way, we could port the whole checkJARMetaData() method to a Gradle task.

https://github.com/apache/lucene/blob/main/dev-tools/scripts/smokeTestRelease.py#L127

@asfimport
Copy link
Author

Dawid Weiss (@dweiss) (migrated from JIRA)

There are a number of things that rely on the java version,Tomoko - when Adrien made a bump to Java 17 we kept discovering that "minimum bar" in various places of the code. Maybe it'd be good to try to cover all of those (or most of those) under one issue? 

As far as I understand - the smoke tester's reason to exist is to have an external, final validation of things. If w tie it to the build then that validation will no longer be independent. Speaking for myself, I think it's fine if we move this check to an integration test within distribution.tests but I recall some discussions asking for the script to be a separate validation check.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

My issue description might not good or correct - I didn't mean to integrate the smoke tester checks into distribution tasks. I understand there could be risks that obscure the boundary between tests and the tested target. My intention was, the smoke tester could utilize Gradle to pick common environments or setup while it is still kept independent from the "distribution" related tasks.

There are a number of things that rely on the java version,Tomoko - when Adrien made a bump to Java 17 we kept discovering that "minimum bar" in various places of the code. Maybe it'd be good to try to cover all of those (or most of those) under one issue?

 
I still don't get the full picture of all the necessary changes about upgrading the java version; I will not be able to seek an optimal solution on it.

As for this issue, as an easy fix, we can bump the java version by just updating all the literal "11" to "17" in the smoke tester script. I think there would be no harm to make the quick fix so that it works on main.

@asfimport
Copy link
Author

Tomoko Uchida (@mocobeta) (migrated from JIRA)

Or - from the extreme perspective - are there any substantial problems/concerns if we fully port the smoke tester to a set of Gradle tasks? (I am even not sure it's possible and/or we want to take time for it, just a thought experiment...)

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit b6c1024 in lucene's branch refs/heads/main from Tomoko Uchida
https://gitbox.apache.org/repos/asf?p=lucene.git;h=b6c1024

LUCENE-10463: increment java version to 17 in smoke tester (#748)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants