Updated to allow passing in maximum number of attempts and poll timeout #72
Conversation
manub
left a comment
manub
left a comment
There was a problem hiding this comment.
Thanks for this @gigaSproule - there's only a couple of small things but it looks good and I'm looking to merge after those changes are addressed.
|
|
||
| import scala.collection.JavaConverters._ | ||
|
|
||
| class ConsumerOpsTest extends EmbeddedKafkaSpecSupport with MockitoSugar { |
There was a problem hiding this comment.
Could you please rename this to ConsumerOpsSpec?
| val consumer: KafkaConsumer[String, String] = mock[KafkaConsumer[String, String]] | ||
| val consumerRecords = new ConsumerRecords[String, String](mapAsJavaMap(Map.empty)) | ||
|
|
||
| when(consumer.poll(1)).thenReturn(consumerRecords) |
There was a problem hiding this comment.
maybe extracting 1 with giving a meaningful value name would be better (here and in subsequent places)
| } | ||
|
|
||
| "not retry to get messages with the configured maximum number of attempts when poll succeeds" in { | ||
| val consumer: KafkaConsumer[String, String] = mock[KafkaConsumer[String, String]] |
There was a problem hiding this comment.
explicit types for in-test values are not necessarily needed
There was a problem hiding this comment.
Yes, sorry, was having difficulties with mocking (was trying ScalaMock, but it wasn't having it)
| Try { | ||
| import scala.collection.JavaConversions._ | ||
| consumer.subscribe(List(topic)) | ||
| import scala.collection.JavaConverters._ |
build.sbt
Outdated
| .settings(publishSettings: _*) | ||
| .settings(commonSettings: _*) | ||
| .settings(commonLibrarySettings) | ||
| .settings(libraryDependencies += "org.mockito" % "mockito-core" % "1.8.5" % Test) |
There was a problem hiding this comment.
Can we use a newer version of Mockito? 1.8.5 is quite old.
There was a problem hiding this comment.
Wow, yes, it really is!
|
As also mentioned in the issue, I'll fix the tests for Scala 2.11 |
…ng a deprecated object
|
seems like |
|
Thanks @gigaSproule, will release in the next days. |
|
Hi @manub, is there any chance of a release including this change, please? It would be useful to me as well. |
|
Sorry, this completely slipped off my radar. Aiming to do this by the weekend. |
|
Released 0.13.0! |
|
Nice one @manub. I've updated my dependencies and this is working. Apologies if this is deliberate on your part, but I noticed that https://github.com/manub/scalatest-embedded-kafka/releases has not been updated to mention the new release, and wondered if you had an issue with your automated release tagging. Hope that's useful to know! |
|
Actually that should have worked - I think something went wrong with the release process and I'm not aware of it. Thanks for pointing it out! |
3 participants
Attempt at fixing issue #71