Skip to content

KAFKA-7708: Fixed KTable tests using KStream API in scala tests#6019

Closed
lodamar wants to merge 1 commit intoapache:trunkfrom
lodamar:KAFKA-7708-fix-ktable-scala-tests
Closed

KAFKA-7708: Fixed KTable tests using KStream API in scala tests#6019
lodamar wants to merge 1 commit intoapache:trunkfrom
lodamar:KAFKA-7708-fix-ktable-scala-tests

Conversation

@lodamar
Copy link
Copy Markdown

@lodamar lodamar commented Dec 10, 2018

After reading https://issues.apache.org/jira/browse/KAFKA-7708 I can confirm KTable scala tests are using KStream API.

I propose this simple PR. I change all builder.stream to builder.table and modify KTable tests accordingly.

KTable scala tests were introduced in: #5502

@lodamar lodamar changed the title Fixed KTable tests using KStream API in scala tests KAFKA-7708 - Fixed KTable tests using KStream API in scala tests Dec 10, 2018
@lodamar lodamar changed the title KAFKA-7708 - Fixed KTable tests using KStream API in scala tests KAFKA-7708: Fixed KTable tests using KStream API in scala tests Dec 10, 2018
Copy link
Copy Markdown
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my gosh, I can't believe I overlooked this before. Thanks for the catch!

@vvcephei
Copy link
Copy Markdown
Contributor

@mjsax , do you mind taking a look, too?

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Dec 11, 2018

Can you give more context? What is wrong with the tests? It should not matter if the table is create from a changelog or from a stream via group-by-aggregate? What do I miss?

@lodamar
Copy link
Copy Markdown
Author

lodamar commented Dec 11, 2018

I read another time old tests and you are right.
My mind read builder.stream and I didn't noticed the final .count() that creates the KTable!

Old tests are right.

@lodamar lodamar closed this Dec 11, 2018
@vvcephei
Copy link
Copy Markdown
Contributor

Ah, well, now I'm doubly embarrassed... Thanks, @mjsax .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants