Skip to content
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

KAFKA-6461 TableTableJoinIntegrationTest is unstable if caching is enabled #4451

Merged
merged 2 commits into from Jan 23, 2018
Merged

Conversation

tedyu
Copy link
Contributor

@tedyu tedyu commented Jan 19, 2018

In test output, NPE is observed on the following line in apply() method:

            if (value.equals(expected)) {

We should check that value is not null before calling equals()

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

Call for second review @bbejeck @dguy @guozhangwang

@@ -79,7 +79,7 @@ public void prepareTopology() throws InterruptedException {
@Override
public void apply(final Long key, final String value) {
numRecordsExpected++;
if (value.equals(expected)) {
if (value != null && value.equals(expected)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can we just call expected.equals(value) which does null check internally as well?

@tedyu
Copy link
Contributor Author

tedyu commented Jan 22, 2018

ResetIntegrationTest failure was not related to the change.

@tedyu
Copy link
Contributor Author

tedyu commented Jan 22, 2018

restest this please.

@guozhangwang
Copy link
Contributor

retest this please

@tedyu
Copy link
Contributor Author

tedyu commented Jan 23, 2018

@mjsax @guozhangwang
Do you have other comments ?

@guozhangwang guozhangwang merged commit faafdbe into apache:trunk Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants