MINOR: Added more integration tests#1285
Conversation
|
@miguno could you have an initial look? |
| testCompile project(':core') | ||
| testCompile project(':core').sourceSets.test.output | ||
| testCompile libs.junit | ||
| testCompile libs.assertj |
There was a problem hiding this comment.
You don't need this to use assertThat notation, hamcrest is included by default in JUnit and we can just use that?
There was a problem hiding this comment.
@ijuma the hamcrest included with junit unfortunately doesn't have the "contains" Matcher included, but I'll use another matcher. Thanks.
| IntegrationTestUtils.produceValuesSynchronously(INPUT_TOPIC_A, inputValues, producerConfig); | ||
|
|
||
| // Give the stream processing application some time to do its work. | ||
| Thread.sleep(10000); |
There was a problem hiding this comment.
Right now I think we should keep the (high) sleep times as is.
But in general we may want to lower the sleep times in this PR. I don't know what a "good" value would be (preferably we'd be able to use TestUtils.waitUntilTrue eventually) -- using sleep just s*cks -- but I think Jenkins should be running a bit faster than Travis. Perhaps we should give it a try with 5s rather than 10s?
That said, we perhaps don't want another iteration on this code, given that the 0.10 vote already started.
There was a problem hiding this comment.
@miguno I tried 5s instead but it didn't always work.
|
LGTM. @guozhangwang : Could you take a look, too, please? |
|
LGTM overall. I'm indeed concerned about the time taken for these tests, as we are addung about one minute into the unit test suite. A couple of things we can do as follow-ups: 1) once #1264 is merged, we can use a mock producer / consumer in Kafka Streams and check directly in the mock producers to check if the expected data was sent, and remove the embedded kafka cluster.
|
|
@guozhangwang I reduced the sleep time quite a bit. Will follow up once other PRs are in. Thanks. |
|
LGTM. Merged to trunk. |
Author: Eno Thereska <eno.thereska@gmail.com> Reviewers: Ismael Juma, Michael G. Noll, Guozhang Wang Closes #1285 from enothereska/more-integration-tests
Author: Eno Thereska <eno.thereska@gmail.com> Reviewers: Ismael Juma, Michael G. Noll, Guozhang Wang Closes apache#1285 from enothereska/more-integration-tests
…to handleReadShareGroupState in KafkaApis.scala (apache#1285)
4 participants
No description provided.