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.

Sign up for GitHub

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

Jump to bottom

Use JUnit 5 and remove JUnit 4 API dependencies #64

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Use JUnit 5 and remove JUnit 4 API dependencies #64

wants to merge 6 commits into from

Conversation

pbielicki
Copy link
Member

No description provided.

@pbielicki pbielicki marked this pull request as draft October 29, 2021 14:54
@pbielicki
Copy link
Member Author

@marcphilipp @britter can you pls check if the direction is sensible?
I primarily focused on replacing junit4 with junit5, so it's not polished but it needs significant changes in the public API, notably @Sample annotation and SamplesRunner that is being removed. It has to be replaced by other extension points. One of the ideas that I propose in this PR is to specify extra features using new @Samples annotation.

@pbielicki pbielicki self-assigned this Oct 29, 2021
marcphilipp
marcphilipp reviewed Nov 2, 2021
View reviewed changes
Copy link
Contributor

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider offering a more gradual migration path from JUnit 4 to 5. We could deprecate the Runner implementation and the old annotations for now and use the same execution logic behind the scenes. WDYT?

samples-check/src/main/java/org/gradle/exemplar/test/engine/SamplesRunnerJUnitEngine.java
}

@Override
public TestDescriptor discover(EngineDiscoveryRequest discoveryRequest, UniqueId uniqueId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discovery should also support UniqueIdSelectors and use EngineDiscoveryRequestResolver for de-duplication.

samples-check/src/main/java/org/gradle/exemplar/test/engine/SamplesRunnerJUnitEngine.java
Function<CommandExecutorParams, CommandExecutor> commandExecutorFunction, List<? extends OutputNormalizer> normalizers,
List<SampleModifier> sampleModifiers
) {
super(uniqueId, sample.getId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pass a FileSource here so that one can navigate to the source in IDEs.

samples-check/src/main/java/org/gradle/exemplar/test/engine/SamplesRunnerJUnitEngine.java
}

@Override
public void execute(ExecutionRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending HierarchicalTestEngine would allow to move the execution logic to the TestDescriptor.

@pbielicki pbielicki force-pushed the przemek/junit-platform-engine branch from d62586d to a9fcbe6 Compare November 2, 2021 08:30
@pbielicki
Copy link
Member Author

I think we should consider offering a more gradual migration path from JUnit 4 to 5. We could deprecate the Runner implementation and the old annotations for now and use the same execution logic behind the scenes. WDYT?

makes perfect sense

@pbielicki pbielicki force-pushed the przemek/junit-platform-engine branch from a9fcbe6 to 0f85578 Compare November 2, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcphilipp marcphilipp marcphilipp left review comments

Assignees

@pbielicki pbielicki

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pbielicki @marcphilipp