You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "agavra (via GitHub)" <gi...@apache.org> on 2024/02/23 02:08:26 UTC

[PR] migrate StreamedJoinTest to Mockito [kafka]

agavra opened a new pull request, #15424:
URL: https://github.com/apache/kafka/pull/15424

   See https://github.com/apache/kafka/pull/14648#discussion_r1499780993 -- turns out I didn't use it because it wasn't included as a build dependency 😆 
   
   cc @ableegoldman @ijuma 
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] migrate StreamedJoinTest to Mockito [kafka]

Posted by "agavra (via GitHub)" <gi...@apache.org>.
agavra commented on code in PR #15424:
URL: https://github.com/apache/kafka/pull/15424#discussion_r1503025048


##########
streams/streams-scala/src/test/scala/org/apache/kafka/streams/scala/kstream/StreamJoinedTest.scala:
##########
@@ -21,24 +21,21 @@ import org.apache.kafka.streams.processor.internals.InternalTopologyBuilder
 import org.apache.kafka.streams.scala.serialization.Serdes
 import org.apache.kafka.streams.scala.serialization.Serdes._
 import org.apache.kafka.streams.state.Stores
-import org.easymock.EasyMock
-import org.easymock.EasyMock.{createMock, replay}
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.{BeforeEach, Test}
+import org.mockito.Mockito
 
 import java.time.Duration
 
 class StreamJoinedTest {

Review Comment:
   the equivalent with JUnit5 is:
   ```
   @ExtendWith(Array(classOf[MockitoExtension]))
   @MockitoSettings(strictness = Strictness.STRICT_STUBS)
   ```
   I've added that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #15424:
URL: https://github.com/apache/kafka/pull/15424#discussion_r1511284102


##########
build.gradle:
##########
@@ -2333,6 +2333,8 @@ project(':streams:streams-scala') {
 
     testImplementation libs.junitJupiter
     testImplementation libs.easymock
+    testImplementation libs.mockitoCore

Review Comment:
   There were no mocks used at all in this module, but the streams module has all but 2 tests using mockito. I removed the easymock dependency from this module (and a few others) in a subsequent commit https://github.com/apache/kafka/commit/2c0cab39aedab3a8635510acfac2551aaeb62ffb



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "ableegoldman (via GitHub)" <gi...@apache.org>.
ableegoldman commented on PR #15424:
URL: https://github.com/apache/kafka/pull/15424#issuecomment-1965688725

   remember to name your PRs correctly -- I added the `KAFKA-15215` prefix since this came about from that ticket, but you can always use `MINOR:` for small fixes that don't have a corresponding ticket


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "ableegoldman (via GitHub)" <gi...@apache.org>.
ableegoldman merged PR #15424:
URL: https://github.com/apache/kafka/pull/15424


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] migrate StreamedJoinTest to Mockito [kafka]

Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on code in PR #15424:
URL: https://github.com/apache/kafka/pull/15424#discussion_r1502590907


##########
streams/streams-scala/src/test/scala/org/apache/kafka/streams/scala/kstream/StreamJoinedTest.scala:
##########
@@ -21,24 +21,21 @@ import org.apache.kafka.streams.processor.internals.InternalTopologyBuilder
 import org.apache.kafka.streams.scala.serialization.Serdes
 import org.apache.kafka.streams.scala.serialization.Serdes._
 import org.apache.kafka.streams.state.Stores
-import org.easymock.EasyMock
-import org.easymock.EasyMock.{createMock, replay}
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.{BeforeEach, Test}
+import org.mockito.Mockito
 
 import java.time.Duration
 
 class StreamJoinedTest {

Review Comment:
   Please add the annotation `@RunWith(MockitoJUnitRunner.StrictStubs.class)` and resolve any errors that it may surface.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "agavra (via GitHub)" <gi...@apache.org>.
agavra commented on code in PR #15424:
URL: https://github.com/apache/kafka/pull/15424#discussion_r1503563459


##########
build.gradle:
##########
@@ -2333,6 +2333,8 @@ project(':streams:streams-scala') {
 
     testImplementation libs.junitJupiter
     testImplementation libs.easymock
+    testImplementation libs.mockitoCore

Review Comment:
   I checked. looks like no tests in the streams scala tests used mockito yet 🤷 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "ableegoldman (via GitHub)" <gi...@apache.org>.
ableegoldman commented on code in PR #15424:
URL: https://github.com/apache/kafka/pull/15424#discussion_r1503562979


##########
build.gradle:
##########
@@ -2333,6 +2333,8 @@ project(':streams:streams-scala') {
 
     testImplementation libs.junitJupiter
     testImplementation libs.easymock
+    testImplementation libs.mockitoCore

Review Comment:
   Wait...why do we need to add a dependency for this? I thought the whole point of the change was to align this test with others in the same file -- shouldn't that mean whatever dependencies are needed are already there?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-15215: migrate StreamedJoinTest to Mockito [kafka]

Posted by "ableegoldman (via GitHub)" <gi...@apache.org>.
ableegoldman commented on PR #15424:
URL: https://github.com/apache/kafka/pull/15424#issuecomment-1965693809

   Looks like a new build was triggered when I renamed the PR but I have the results from before that and it looked good. Test failures all unrelated. Will merge this to trunk
   
   Build from before the name change: https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15424/2/tests/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org