-
Notifications
You must be signed in to change notification settings - Fork 77
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
Upgrade to spring boot 2.7.6, junit 5, gradle 7.5.1 #125
Conversation
f692222
to
a828bd8
Compare
@@ -1,6 +1,6 @@ | |||
package io.github.stavshamir.springwolf.asyncapi.types.channel.operation.message.header; | |||
|
|||
import org.springframework.kafka.support.converter.AbstractJavaTypeMapper; | |||
import org.springframework.kafka.support.mapping.AbstractJavaTypeMapper; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to the spring update, also spring-kafka is updated, which includes this change.
If we are assuming that springwolf users are up-to-date, then this change is good. Otherwise, this might force users to upgrade spring-kafka.
I am not sure if we want to include in the next release as it contains already so many changes: async annotations + spring properties + spring-cloud + fixes.
Or, this could be a minor update right after the upcoming release (1.1.0)
FYI: I also started looking into upgrade to spring boot 3, but run in some issues with the new cloud stream plugin. |
a828bd8
to
5502547
Compare
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.nio.charset.StandardCharsets; | ||
import java.util.Map; | ||
|
||
@RunWith(SpringRunner.class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JUnit5 equivalent would be @ExtendWith(SpringExtension.class)
. @SpringBootTest
would work as well, but will make the test much slower
|
||
|
||
@RunWith(Enclosed.class) | ||
@Nested |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought @Nested
only has meaning in inner classes. What does it do when it's top level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this, feels good to get rid of JUnit4 and the kotlin tests!
Only one important comment - replacing JUnit4 @RunWith(SpringRunner.class)
with @SpringBootTest
will work but make tests much slower since Spring will load the entire context. Except for context and integration test, we should use @ExtendWith(SpringExtension.class)
instead.
- @nested is not needed with junit5 - Use @ExtendWith(SpringExtension.class) instead of @SpringBootTest, which is faster
Also upgrade other dependencies
Remove kotlin (converted test to java)
Relates to: #112
Relates to: #110