Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a custom JUnit test engine for discovering and executing resource-based tests. The engine integrates with the JUnit Platform to enable test discovery from configuration files located in directories, allowing sample projects to be tested programmatically.
Key changes include:
- New
samples-junit-enginemodule with custom test engine implementation - Extended
Samplemodel to include config file reference for better test identification - Updated Gradle wrapper to snapshot version 9.4.0
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| settings.gradle.kts | Adds new samples-junit-engine module to the build |
| samples-junit-engine/build.gradle.kts | Configures the new module with JUnit engine dependencies and integration test suite |
| samples-junit-engine/src/main/java/org/gradle/exemplar/ExemplarTestEngine.java | Main test engine implementation that discovers and executes sample tests |
| samples-junit-engine/src/main/java/org/gradle/exemplar/ExemplarTestResolver.java | Resolves directory selectors into executable test descriptors |
| samples-junit-engine/src/main/java/org/gradle/exemplar/ExemplarTestDescriptor.java | Describes individual sample tests for the JUnit platform |
| samples-junit-engine/src/main/resources/META-INF/services/org.junit.platform.engine.TestEngine | Service provider configuration for test engine discovery |
| samples-junit-engine/src/test-definitions/my-test-project/* | Sample test definitions for integration testing |
| samples-junit-engine/src/integTest/java/test/* | Test utilities for modifiers and normalizers used during testing |
| samples-discovery/src/main/java/org/gradle/exemplar/model/Sample.java | Extends Sample model to include optional config file reference |
| samples-discovery/src/main/java/org/gradle/exemplar/loader/SamplesDiscovery.java | Updates sample loading to pass config file to Sample constructor |
| gradle/wrapper/* | Updates Gradle wrapper to version 9.4.0 snapshot |
| gradle/libs.versions.toml | Adds JUnit platform engine and Jupiter dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| private static <T> void populateFromSystemProperty(ExecutionRequest request, String propertyName, Set<T> targetSet) { | ||
| Optional<String> values = request.getConfigurationParameters().get(propertyName); | ||
| Boolean hasValues = values.isPresent() && !values.get().trim().isEmpty(); |
There was a problem hiding this comment.
The variable name 'hasValues' (line 166) uses Boolean wrapper type, but it's immediately assigned a boolean primitive expression. Consider using primitive 'boolean' type instead of 'Boolean' wrapper for better performance and to avoid unnecessary boxing.
| Boolean hasValues = values.isPresent() && !values.get().trim().isEmpty(); | |
| boolean hasValues = values.isPresent() && !values.get().trim().isEmpty(); |
| public Resolution resolve(DirectorySelector selector, Context context) { | ||
| LOGGER.info(() -> "Test specification dir: " + selector.getDirectory().getAbsolutePath()); | ||
| List<Sample> samples = SamplesDiscovery.externalSamples(selector.getDirectory()); | ||
| Set<DiscoverySelector> selectors = new LinkedHashSet<>(); |
There was a problem hiding this comment.
The unused local variable 'selectors' is declared and initialized but never used. This appears to be leftover code that should be removed.
| Set<DiscoverySelector> selectors = new LinkedHashSet<>(); |
|
|
||
| # | ||
| # Copyright © 2015-2021 the original authors. | ||
| # Copyright © 2015 the original authors. |
There was a problem hiding this comment.
The copyright year range has been changed from "2015-2021" to just "2015". If this is intentional to simplify the copyright statement, it's acceptable. However, if the file has been modified in years after 2015, the broader range would be more accurate.
| # Copyright © 2015 the original authors. | |
| # Copyright © 2015-2021 the original authors. |
donat
force-pushed
the
donat/junit-test-engine
branch
from
0a58f27 to
1be2b46
Compare
donat
force-pushed
the
donat/junit-test-engine
branch
from
1be2b46 to
9f2c1c7
Compare
samples-junit-engine/src/main/java/org/gradle/exemplar/ExemplarTestEngine.java
Outdated
Show resolved
Hide resolved
|
I've improved the test event hierarchy a bit. |
Image isn't clickable. "This private-user-images.githubusercontent.com page can’t be found" |
|
|
||
| private static File createTmpDir(ExecutionRequest request) { | ||
| try { | ||
| Optional<String> s = request.getConfigurationParameters().get("exemplar.tmpdir"); |
| import java.util.*; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class ExemplarTestEngine implements TestEngine { |
There was a problem hiding this comment.
🤔 Kind of a lot is done directly by this engine class. Maybe it would be better to split into:
- Overall Testing Setup (createTmpDir, etc)
- Individual Test Directory Creation and Test Configuration (modifiers)
- Test Execution (execute, leave in engine)
- Test Verification (verifyOutput, perhaps also leave in engine)
I think the inidividual test configuration bit is the noisiest, and it isn't really "TestEngine" code that JUnit cares about.
2 participants
Discover and run tests via a custom, resource-based JUnit test engine.