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.

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

[WIP] JUnit 5 #677

Open
wants to merge 4 commits into
base: master
from

Conversation

@sormuras
Copy link
Contributor

commented Sep 13, 2018

in progress...

  • Use JUnit Platform at test runtime
  • Migrate single test to use JUnit Jupiter API
  • Don't over-use the Jupiter API, tune it down a bit

Related to #603 and junit-team/junit5#1582

@JakeWharton

This comment has been minimized.

Copy link
Collaborator

commented Sep 13, 2018

JavaPoet is imported as source into Google's monorepo so I'll have to check as to whether JUnit 5 is supported or not. Or maybe @ronshapiro knows immediately off-hand (I'm inclined to think it's not supported).

My favorite quote from the linked issue is:

they are two separate frameworks

JUnit 5 was named incorrectly. This is an entirely different framework that should be vetted on its own merits compared to JUnit 4. It is decidedly not an upgrade.

So we should evaluate it against other new frameworks and also against doing nothing.

@ronshapiro

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2018

It's not, but we also don't run JavaPoet's tests (for a silly reason, I think at the time none of the tests had @RunWith and our runner didn't support detecting them (over JUnit3 tests)).

But please don't make a decision based on Google's eng structure (especially for tests!) - that seems unfair.

Use JUnit Platform
Prior to this commit Maven Surefire selected the provided `junit4`
provider to find and execute JUnit 4 tests. Now the JUnit Vintage
(read: Good Ol') Engine is in charge to find and execute tests.
Surefire 2.22.0 natively supports the JUnit Platform and all
available TestEngine implementations at runtime.

@sormuras sormuras force-pushed the sormuras:junit5 branch from db0abad to aa9c4d1 Sep 13, 2018

@sormuras sormuras force-pushed the sormuras:junit5 branch from d299aff to 1ce1bf6 Sep 13, 2018

@swankjesse

This comment has been minimized.

Copy link
Member

commented Sep 24, 2018

Yep. Because JUnit 4 and JUnit 5 don’t interoperate nicely I think we’re better off waiting until all of our dependent projects have upgraded. In particular, we should wait for compile-testing and Mockito to support JUnit 5.

import org.junit.jupiter.api.DynamicNode;
import org.junit.jupiter.api.DynamicTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestFactory;

This comment has been minimized.

Copy link
@swankjesse

swankjesse Sep 24, 2018

Member

Going from 2 imports to 9 makes me want to back away from Jupiter forever.

This comment has been minimized.

Copy link
@sormuras

sormuras Sep 24, 2018

Author Contributor

+7 more imports to go from

image

to

image

Better reporting needs some imports.

But as I stated in the original description text, I'll rework this commit to only show minimal changes.

</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId> <!-- junit-jupiter-api, Surefire is not smart enough, yet. -->

This comment has been minimized.

Copy link
@swankjesse

swankjesse Sep 24, 2018

Member

Blech. They say one of Jupiter’s benefits is better integration with test runners. But I see that it isn’t integrated with Maven yet. That doesn’t inspire confidence.

This comment has been minimized.

Copy link
@sormuras

sormuras Sep 24, 2018

Author Contributor

Blech.

Hey, I understand that. (-:

Come over and help improving Surefire!

Or try https://github.com/sormuras/junit-platform-maven-plugin which already "is smart enough" to add required runtime dependencies.

@sormuras

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

JUnit 4 and JUnit 5 don’t interoperate nicely

It's coming along...

junit-team/junit5#1586
junit-team/junit5@c4533c3

Mockito

Mockito 2.22.0 already provides a MockitoExtension: https://static.javadoc.io/org.mockito/mockito-junit-jupiter/2.22.0/org/mockito/junit/jupiter/MockitoExtension.html

sormuras added 2 commits Sep 24, 2018
Convert LineWrapperTest to use Jupiter API
From 3 imports down to 1 and erased many `public` modifiers.
Convert NameAllocatorTest to use Jupiter API
From 6 to 2 lines using `Assertions.assertThrows` instead of
the `try { fail() } catch ( expected )` pattern.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.