Skip to content

HOTFIX: StateDirectoryTest should use Set instead of List#8305

Merged
mjsax merged 1 commit intoapache:trunkfrom
abbccdda:hotfix
Mar 17, 2020
Merged

HOTFIX: StateDirectoryTest should use Set instead of List#8305
mjsax merged 1 commit intoapache:trunkfrom
abbccdda:hotfix

Conversation

@abbccdda
Copy link
Copy Markdown

@abbccdda abbccdda commented Mar 16, 2020

This PR fixes the undefined order of list directory result, to make the test result consistent, otherwise we would hit out of order from time to time:

java.lang.AssertionError: expected:<[/tmp/kafka-k0Nn0/applicationId/0_0, /tmp/kafka-k0Nn0/applicationId/0_1]> but was:<[/tmp/kafka-k0Nn0/applicationId/0_1, /tmp/kafka-k0Nn0/applicationId/0_0]>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:120)
	at org.junit.Assert.assertEquals(Assert.java:146)
	at org.apache.kafka.streams.processor.internals.StateDirectoryTest.shouldOnlyListNonEmptyTaskDirectories(StateDirectoryTest.java:318)

Committer Checklist (excluded from commit message)

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

@abbccdda abbccdda force-pushed the hotfix branch 2 times, most recently from 143661d to 0f0c8ed Compare March 16, 2020 23:31
mjsax
mjsax approved these changes Mar 16, 2020
View reviewed changes
Copy link
Copy Markdown
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.

LGTM. Thanks for the quick fix!

@mjsax mjsax added streams tests Test fixes (including flaky tests) labels Mar 16, 2020
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 16, 2020

Retest this please.

guozhangwang
guozhangwang approved these changes Mar 16, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread streams/src/test/java/org/apache/kafka/streams/processor/internals/StateDirectoryTest.java Outdated
Copy link
Copy Markdown
Contributor

@guozhangwang guozhangwang Mar 16, 2020

Choose a reason for hiding this comment

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

Good call.

@abbccdda
Copy link
Copy Markdown
Author

abbccdda commented Mar 17, 2020

Btw, I guess the build won't pass as current trunk has another breakage caused by request quota, will wait and see when that one gets resolved @guozhangwang @mjsax

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 17, 2020

Retest this please

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 17, 2020

Seems the push -f killed the build -- not sure why I could not restart it -- also, I rebased to trunk recently and it worked -- seems all fixes got merged already?

@abbccdda
Copy link
Copy Markdown
Author

abbccdda commented Mar 17, 2020

@mjsax The fix is merged

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 17, 2020

Cool. Build is running now. :)

@chia7712 chia7712 mentioned this pull request Mar 17, 2020
Closed
3 tasks
@mjsax mjsax merged commit 1e6d944 into apache:trunk Mar 17, 2020
ijuma added a commit to confluentinc/kafka that referenced this pull request Mar 17, 2020
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into ccs-master
6f7084a 
* apache-github/trunk: (39 commits)
  MINOR: cleanup and add tests to StateDirectoryTest (apache#8304)
  HOTFIX: StateDirectoryTest should use Set instead of List (apache#8305)
  MINOR: Fix build and JavaDoc warnings (apache#8291)
  MINOR: Fix kafka.server.RequestQuotaTest missing new ApiKeys. (apache#8302)
  KAFKA-9712: Catch and handle exception thrown by reflections scanner (apache#8289)
  KAFKA-9670; Reduce allocations in Metadata Response preparation (apache#8236)
  MINOR: fix Scala 2.13 build error introduced in apache#8083 (apache#8301)
  MINOR: enforce non-negative invariant for checkpointed offsets (apache#8297)
  MINOR: comment apikey types in generated switch (apache#8201)
  MINOR: Fix typo in CreateTopicsResponse.json (apache#8300)
  KIP-546: Implement describeClientQuotas and alterClientQuotas. (apache#8083)
  KAFKA-6647: Do note delete the lock file while holding the lock (apache#8267)
  KAFKA-9677: Fix consumer fetch with small consume bandwidth quotas (apache#8290)
  KAFKA-9533: Fix JavaDocs of KStream.transformValues (apache#8298)
  MINOR: reuse pseudo-topic in FKJoin (apache#8296)
  KAFKA-6145: Pt 2. Include offset sums in subscription (apache#8246)
  KAFKA-9714; Eliminate unused reference to IBP in `TransactionStateManager` (apache#8293)
  KAFKA-9718; Don't log passwords for AlterConfigs in request logs (apache#8294)
  KAFKA-8768: DeleteRecords request/response automated protocol (apache#7957)
  KAFKA-9685: Solve Set concatenation perf issue in AclAuthorizer
  ...
guozhangwang pushed a commit that referenced this pull request Mar 18, 2020
@guozhangwang
HOTFIX: StateDirectoryTest should use Set instead of List (#8305)
89a5355 
Reviewers: Matthias J. Sax <matthias@confluent.io>, Guozhang Wang <guozhang@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mjsax mjsax mjsax approved these changes

@guozhangwang guozhangwang guozhangwang approved these changes

Assignees

No one assigned

Labels

streams tests Test fixes (including flaky tests)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@abbccdda @mjsax @guozhangwang