-
Notifications
You must be signed in to change notification settings - Fork 73
Add support for JUnit's @ParameterizedClass
#711
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 support for JUnit's @ParameterizedClass
#711
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
@@ -54,6 +54,7 @@ dependencies { | |||
implementation(libs.openjson) | |||
testImplementation(platform(libs.test.junit.bom)) | |||
testImplementation(libs.test.junit.jupiter.core) | |||
testRuntimeOnly(libs.test.junit.platform.launcher) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forces Gradle to use an aligned version of junit-platform-launcher
. See https://junit.org/junit5/docs/current/user-guide/#running-tests-build-gradle-bom for details.
includeGroup("org.junit") | ||
includeGroupByRegex("org\\.junit\\..*") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only temporary until there's a release (candidate?) on Maven Central.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add such valuable comments to the commit message bodies instead of the PR discussion?
"org.junit.jupiter.engine.config.InstantiatingConfigurationParameterConverter", | ||
// new in Junit 5.13? | ||
"org.junit.jupiter.api.DisplayNameGenerator$IndicativeSentences", | ||
"org.junit.jupiter.engine.discovery.ClassSelectorResolver" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are also temporary since they have been added to the native-image.properties
that ship with JUnit in the meantime.
@@ -61,6 +61,7 @@ public abstract static class ArgumentTestBase { | |||
@BeforeAll | |||
public static void setup() { | |||
actualArgs.clear(); | |||
expectedArgs.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lifecycle method was never called in JUnit 5.11 and earlier. See https://junit.org/junit5/docs/5.11.0/user-guide/index.html#extensions-supported-utilities-search-semantics for details.
This led to both actual and expected args to accumulate which passed the assertions in the @AfterAll
method below. Now that this method is called, these assertions failed for all but the first executed subclass.
@ParameterizedClass
@ParameterizedClass
@@ -0,0 +1,84 @@ | |||
/* | |||
* Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this just be 2025?
@@ -155,6 +156,11 @@ private void registerTestClassForReflection(Class<?> clazz) { | |||
if (superClass != null && superClass != Object.class) { | |||
registerTestClassForReflection(superClass); | |||
} | |||
for (var declaredClass : clazz.getDeclaredClasses()) { | |||
if (!Modifier.isStatic(declaredClass.getModifiers())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, one could also look for @Nested
but that would mean a closer coupling. Thoughts?
Thank you for signing the OCA. |
Superseded by #693 |
JUnit 5.13 will introduce parameterized test classes which also allow
@Nested
test classes inside of them, but those are not part of theTestPlan
and thus not registered for reflection.This PR addresses that by registering all inner classes of test classes for reflection.
@Nested
tests in@ParameterizedClass
es junit-team/junit5#4440