Spike for expression language evaluation in display names #4293

jean-andre-gauthier · 2025-01-31T14:20:25Z

Overview

Introduces a new @ExpressionLanguage annotation, that takes a Class<ExpressionLanguageAdapter> as parameter
Introduces a new ExpressionLanguageAdapter interface, that can be implemented to bind in a third-party expression language
Adds several tests in ExpressionLanguageMustacheTests, that demonstrate how @ExpressionLanguage / ExpressionLanguageAdapter can be used

I'm marking this as a draft PR, because it's not entirely clear yet how the API should look like. I'll address the items in the Definition of Done once we have more clarity on that.

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes

…TestMethodContext

…stExtensionTests to TestExtensionContext

…erFormatter so that a custom expression language formatter can be plugged in

…ameFormatter, pull ArgumentsContext up to separate class

jean-andre-gauthier · 2025-01-31T14:29:21Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestNameFormatter.java

+
+		@Override
+		public void append(ArgumentsContext argumentsContext, StringBuffer result) {
+			expressionLanguageAdapter.format(argumentsContext.arguments.get()[0], result);


This is not correct, but I'm unsure how we should name multiple arguments, so that they can be accessed by the EL?

I think we should provide a Map<String, Object> using the method parameter names as keys. That would require compiling with javac's -parameters option. We already check for parameter.isNamePresent() in ParameterizedTestMethodContext.getParameterName(int), though, so we could populate the Map based on that and fail (?) if no parameter names are present?

Regarding the Map<String, Object>, that's also what I suggested in #1154 (comment).

In summary, I believe it would be better to design a new extension API that receives the String displayName and a Map<String,Object> expressionContextMap and then allow people to pick between Mustache, SpEL, etc.

That would require compiling with javac's -parameters option. We already check for parameter.isNamePresent() in ParameterizedTestMethodContext.getParameterName(int), though, so we could populate the Map based on that and fail (?) if no parameter names are present?

I don't think I would throw an exception. Rather, I would document the behavior, pointing out that source code parameter names are available when compiled with -parameters and that otherwise the arguments are available via arg0, arg1, etc. (i.e., the default parameter names returned by Parameter.getName()).

sbrannen

I've only taken a quick glance, but for starters...

This new feature must be applicable to JUnit Jupiter in general, not just for parameterized tests.

So the APIs need to reside in the junit-jupiter-api module.

Please revise your work to take that into account.

See also: #1154 (comment)

marcphilipp · 2025-02-06T09:43:21Z

jupiter-tests/jupiter-tests.gradle.kts

@@ -25,6 +25,7 @@ dependencies {
 	testImplementation(testFixtures(projects.junitJupiterEngine))
 	testImplementation(testFixtures(projects.junitPlatformLauncher))
 	testImplementation(testFixtures(projects.junitPlatformReporting))
+	implementation(libs.mustache)


Suggested change

implementation(libs.mustache)

testImplementation(libs.mustache)

marcphilipp · 2025-02-06T09:43:33Z

gradle/libs.versions.toml

@@ -55,6 +55,7 @@ mavenSurefirePlugin = { module = "org.apache.maven.plugins:maven-surefire-plugin
 memoryfilesystem = { module = "com.github.marschall:memoryfilesystem", version = "2.8.1" }
 mockito-core = { module = "org.mockito:mockito-core", version.ref = "mockito" }
 mockito-junit-jupiter = { module = "org.mockito:mockito-junit-jupiter", version.ref = "mockito" }
+mustache = { module = "com.github.spullara.mustache.java:compiler", version = "0.9.10"}


Suggested change

mustache = { module = "com.github.spullara.mustache.java:compiler", version = "0.9.10"}

mustache = { module = "com.github.spullara.mustache.java:compiler", version = "0.9.10" }

marcphilipp · 2025-02-06T09:49:27Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestNameFormatter.java

+				try {
+					ExpressionLanguageAdapter adapterInstance = adapterClass.getDeclaredConstructor().newInstance();
+					adapterInstance.compile(segment);
+					return adapterInstance;
+				} catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException ex) {
+					String message = "Failed to initialize expression language for parameterized test. "
+							+ "See nested exception for further details.";
+					throw new JUnitException(message, ex);
+				}


Suggested change

try {

ExpressionLanguageAdapter adapterInstance = adapterClass.getDeclaredConstructor().newInstance();

adapterInstance.compile(segment);

return adapterInstance;

} catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException ex) {

String message = "Failed to initialize expression language for parameterized test. "

+ "See nested exception for further details.";

throw new JUnitException(message, ex);

}

ExpressionLanguageAdapter adapterInstance = ReflectionSupport.newInstance(adapterClass);

adapterInstance.compile(segment);

return adapterInstance;

marcphilipp · 2025-02-06T09:57:42Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedTestNameFormatter.java

+
+		@Override
+		public void append(ArgumentsContext argumentsContext, StringBuffer result) {
+			expressionLanguageAdapter.format(argumentsContext.arguments.get()[0], result);


I think we should provide a Map<String, Object> using the method parameter names as keys. That would require compiling with javac's -parameters option. We already check for parameter.isNamePresent() in ParameterizedTestMethodContext.getParameterName(int), though, so we could populate the Map based on that and fail (?) if no parameter names are present?

marcphilipp · 2025-02-06T10:00:52Z

junit-jupiter-params/src/main/java/org/junit/jupiter/params/ExpressionLanguageAdapter.java

+	void compile(String template);
+
+	void format(Object scope, StringBuffer result);


I think we should split this into two to allow (not necessarily in this PR) registering a global default expression language that would support instantiating the ExpressionLanguageAdapter at most once and also provide a hook to clean up resources by extending AutoCloseable.

Suggested change

void compile(String template);

void format(Object scope, StringBuffer result);

Expression compile(String template);

interface Expression { void evaluate(Map<String, Object> context, Appendable result); }

marcphilipp · 2025-02-06T10:03:58Z

This new feature must be applicable to JUnit Jupiter in general, not just for parameterized tests.

@sbrannen Besides parameterized tests, what other use cases with "dynamic display names" did you have in mind? @RepeatedTest(name = ...)? Dynamic tests?

sbrannen · 2025-02-06T10:52:45Z

@sbrannen Besides parameterized tests, what other use cases with "dynamic display names" did you have in mind? @RepeatedTest(name = ...)? Dynamic tests?

There were discussions in #1154, #2452, and other places over the years where people expressed an interest in general-purpose expression language support.

I think at the very least that this should be usable with any @TestTemplate. Though, we may find this can be used in other scenarios as well.

And I fear that tying this to junit-jupiter-param would unnecessarily limit our options in this area.

jean-andre-gauthier · 2025-02-07T16:23:08Z

Hi both, thanks very much for your reviews! I'll be off on holidays for the next two weeks, but will work on implementing #1154 (comment) after that (once it has been discussed with the team)

kaipe · 2025-02-09T16:24:06Z

@sbrannen Besides parameterized tests, what other use cases with "dynamic display names" did you have in mind? @RepeatedTest(name = ...)? Dynamic tests?

There were discussions in #1154, #2452, and other places over the years where people expressed an interest in general-purpose expression language support.

I think at the very least that this should be usable with any @TestTemplate. Though, we may find this can be used in other scenarios as well.

And I fear that tying this to junit-jupiter-param would unnecessarily limit our options in this area.

Here I like to remind about my opinion on the subject #4246 (comment) - as a way to not "limit our options in this area".

I.e. not tying it to junit-jupiter-param nor junit-jupiter-engine but offering it from junit-platform-engine and allow display-name manipulation in midair during test-execution.

It would allow JUnit Jupiter to expose an API for display-name manipulation as part of ExtensionContext, therewith allowing any kind of Jupiter extension to manipulate display-name with some expression language or any other mean available. Perhaps with a popup dialog, in which a tester could document how an exploratory test session caused a backend error?

There is a kind of implementation for it, because some level of this functionality is required by LazyParams.

jean-andre-gauthier added 7 commits December 6, 2024 14:00

Add @ExpressionLanguage annotation, and reference it in Parameterized…

a40da53

…TestMethodContext

Reformat code with eclipse formatter

e9c1619

Extract getExtensionContextReturningSingleMethod from ParameterizedTe…

ab3ed37

…stExtensionTests to TestExtensionContext

Replace logic in ParameterizedTestNameFormatter.determineNonPlacehold…

ba7b30c

…erFormatter so that a custom expression language formatter can be plugged in

Wire up logic to compile & parse EL expressions in ParameterizedTestN…

22c697b

…ameFormatter, pull ArgumentsContext up to separate class

Fix logic to extract EL arguments

e3963d9

Add additional tests for mustache EL

fdbe111

jean-andre-gauthier commented Jan 31, 2025

View reviewed changes

marcphilipp self-requested a review January 31, 2025 14:54

marcphilipp linked an issue Jan 31, 2025 that may be closed by this pull request

Introduce extension API for expression language evaluation in display names #1154

Open

2 tasks

marcphilipp added type: enhancement component: Jupiter theme: parameterized tests labels Jan 31, 2025

sbrannen requested changes Jan 31, 2025

View reviewed changes

marcphilipp reviewed Feb 6, 2025

View reviewed changes

marcphilipp mentioned this pull request Feb 7, 2025

Introduce extension API for expression language evaluation in display names #1154

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spike for expression language evaluation in display names #4293

Spike for expression language evaluation in display names #4293

jean-andre-gauthier commented Jan 31, 2025

jean-andre-gauthier Jan 31, 2025

marcphilipp Feb 6, 2025

sbrannen Feb 6, 2025

sbrannen Feb 6, 2025

sbrannen left a comment

marcphilipp Feb 6, 2025

marcphilipp Feb 6, 2025

marcphilipp Feb 6, 2025

marcphilipp Feb 6, 2025

marcphilipp Feb 6, 2025

marcphilipp commented Feb 6, 2025

sbrannen commented Feb 6, 2025

jean-andre-gauthier commented Feb 7, 2025

kaipe commented Feb 9, 2025 •

edited

Loading

	implementation(libs.mustache)
	testImplementation(libs.mustache)

	mustache = { module = "com.github.spullara.mustache.java:compiler", version = "0.9.10"}
	mustache = { module = "com.github.spullara.mustache.java:compiler", version = "0.9.10" }

		void compile(String template);

		void format(Object scope, StringBuffer result);

Spike for expression language evaluation in display names #4293

Are you sure you want to change the base?

Spike for expression language evaluation in display names #4293

Conversation

jean-andre-gauthier commented Jan 31, 2025

Overview

Definition of Done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbrannen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcphilipp commented Feb 6, 2025

sbrannen commented Feb 6, 2025

jean-andre-gauthier commented Feb 7, 2025

kaipe commented Feb 9, 2025 • edited Loading

kaipe commented Feb 9, 2025 •

edited

Loading