Migrate from JUnit 4 to JUnit 5 #1652
Conversation
|
Can one of the admins verify this patch? |
kunal-kushwaha
force-pushed
the
MigrateToJUnit5
branch
from
cc919d6
to
ff3c656
Compare
|
ok to test |
|
@kunal-kushwaha : Could you please resolve conflicts here also? |
|
@ricardozanini : I see that you wanted to contribute to this issue. Would really appreciate if you could help to review this PR ;-) . This PR is sent by a Google summer of code student so would really appreciate if you contribute here(if you have time of course ;-) ) |
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-migrationsupport</artifactId> | ||
| <version>${junit.version}</version> |
There was a problem hiding this comment.
Did you mean junit.version here or junit.platform.version
There was a problem hiding this comment.
@rohanKanojia: I changed <junit.version> to 5.5.1 used in vintage and jupiter.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.jupiter</groupId> | ||
| <artifactId>junit-jupiter-migrationsupport</artifactId> |
There was a problem hiding this comment.
again, did you really mean to use junit.version here?
There was a problem hiding this comment.
@rohanKanojia: Yes, it's the latest version 5.5.1 for org.junit.jupiter.
extensions/knative/client/pom.xml
Outdated
| </dependency> | ||
| <dependency> | ||
| <groupId>org.junit.vintage</groupId> | ||
| <artifactId>junit-vintage-engine</artifactId> |
There was a problem hiding this comment.
Hi! Why we need to use the vintage engine? We still depends on projects that run on JUnit 4?
There was a problem hiding this comment.
Hi, I put it just for testing but it's not required. Fixed.
extensions/knative/tests/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.junit.vintage</groupId> | ||
| <artifactId>junit-vintage-engine</artifactId> | ||
| <version>${junit.version}</version> |
There was a problem hiding this comment.
I'd say to have one single property specification junit.version or junit.platform.version.
| public void testIllegalNodeName() throws JsonProcessingException { | ||
| new io.fabric8.kubernetes.api.model.NodeBuilder().withNewMetadata().withName("..").and().build(); | ||
| Assertions.assertThrows(ConstraintViolationException.class, new Executable() { |
There was a problem hiding this comment.
This is just a question, since I had the same problem and solved by calling the method directly. Why we can't just call new io.fabric8.kubernetes.api.model.NodeBuilder().withNewMetadata().withName("..").and().build(); directly in the Assertions?
There was a problem hiding this comment.
Actually, the source version in the model was set to 1.7 which does not allow Lambda Expressions. Fixed.
kubernetes-server-mock/pom.xml
Outdated
| <plugins> | ||
| <plugin> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <version>3.8.1</version> |
There was a problem hiding this comment.
Can't we have this version on a properties section inside a BOM?
kubernetes-server-mock/pom.xml
Outdated
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <version>2.22.2</version> |
kunal-kushwaha
force-pushed
the
MigrateToJUnit5
branch
from
fb63d3f
to
1bad83a
Compare
|
@kunal-kushwaha : Could you please rebase your branch with latest master? I think we are good to merge this. |
… into MigrateToJUnit5
kunal-kushwaha
force-pushed
the
MigrateToJUnit5
branch
from
e5b9126
to
3dd197c
Compare
|
@rohanKanojia: fixed conflicts |
|
[merge] |
Fixes #1331