Upgrade dependencies, Upgrade to JDK17 #2747
Conversation
|
@hduerkop - the minimum JDK version of TestNG cannot yet be more than 11. We havent yet released one version that uses jdk11 as default minimum version. So please exclude the JDK bump from your changeset and update this PR. |
testng-core/src/test/java/org/testng/internal/invokers/ParameterHandlerTest.java
Outdated
Show resolved
Hide resolved
| import org.testng.IReporter; | ||
| import org.testng.ITestNGListener; | ||
| import org.testng.TestNG; | ||
| import org.testng.annotations.AfterClass; | ||
| import org.testng.annotations.BeforeClass; | ||
| import org.testng.annotations.DataProvider; | ||
| import org.testng.annotations.Test; | ||
| import org.testng.reporters.EmailableReporter; | ||
| import org.testng.reporters.EmailableReporter2; | ||
| import test.SimpleBaseTest; | ||
|
|
||
| public class EmailableReporterTest extends SimpleBaseTest { |
There was a problem hiding this comment.
Same goes for this as well. Any specific reason behind us removing off this test that works with a custom security manager?
There was a problem hiding this comment.
I removed it because of https://openjdk.java.net/jeps/411 .
There was a problem hiding this comment.
@hduerkop - Please revert it back since we are still saying that we are going to be JDK11 as the minimum JDK but the security manager has been deprecated in JDK17. So we should perhaps leave it there as is until TestNG moves into JDK17 or above as its minimum JDK.
There was a problem hiding this comment.
I have to disagree here since JDK17 is the current LTS and is or soon will be dominant on build platforms
There was a problem hiding this comment.
@hduerkop - if testng has jdk11 as its minimum jdk then we will need to have everything that exists in jdk11 including tests that test features which are relevant for jdk11 as well.
I agree that build systems atleast on the open source world may move forward but we need to be cognizant of user needs and cannot enforce this upon our users.
There was a problem hiding this comment.
It is not an enforcement, but an option to use JDK17 this way - 11 and 17 work this way.
There was a problem hiding this comment.
What was the initial objective of the test? Is it another way to test it?
|
|
||
| tasks.withType<KotlinCompile>().configureEach { | ||
| kotlinOptions { | ||
| jvmTarget = "11" |
There was a problem hiding this comment.
Is it possible to manage this version in the gradle properties file?
There was a problem hiding this comment.
I don't think so, but feel free to improve it
|
@krmahadevan do you plan a release soon ? |
|
@hduerkop - Can you please help fix the merge conflicts so that this PR can be merged ? |
@juherr - We can do that. 7.5 was released in Jan and we do have some bug fixes that can be released along with the JDK11 requirements as well. I will wait for this PR to be mergeable (after conflicts have been resolved) and then maybe we can go through with a release. |
|
@krmahadevan it looks mergeable to me. |
Fixes # .
Did you remember to?
CHANGES.txt./gradlew autostyleApplyWe encourage pull requests that:
If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.
Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.