Skip to content

KAFKA-12892: Use dedicated root in ZK ACL test#10821

Merged
omkreddy merged 1 commit intoapache:trunkfrom
soarez:kafka-12892
Jun 5, 2021
Merged

KAFKA-12892: Use dedicated root in ZK ACL test#10821
omkreddy merged 1 commit intoapache:trunkfrom
soarez:kafka-12892

Conversation

@soarez
Copy link
Member

@soarez soarez commented Jun 4, 2021

Having the testChrootExistsAndRootIsLocked test in a separate ZookeeperTestHarness isn't enough to prevent the ACL changes to the ZK root from affecting other integration tests. So instead, let's use a dedicated znode for this test. It still works because makeSurePersistentPathExists uses createRecursive, which will recurse and act the same for the root or a given znode.

Changes:

  • Use a dedicated znode for the test
  • Move the test to KafkaZkClientTest - there is no longer a need to
    keep it in a separate ZooKeeperTestHarness

Related: #10820

Committer Checklist (excluded from commit message)

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

Changes:

- Use a dedicated znode for the test
- Move the test to `KafkaZkClientTest` - there is no longer a need to
keep it in a separate `ZooKeeperTestHarness`
@showuon
Copy link
Member

showuon commented Jun 5, 2021

@soarez , thanks for the quick fix. But unfortunately, the PR build still failed for "JDK 15 and Scala 2.13" build. I don't see the AclException in the log, but still failed the build. FYI.

@omkreddy
Copy link
Contributor

omkreddy commented Jun 5, 2021

Changes LGTM. let us re-run the tests.

Copy link
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

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

@soarez Thanks for the PR. LGTM

@omkreddy omkreddy merged commit 37a8659 into apache:trunk Jun 5, 2021
sakibguy added a commit to sakibguy/kafka that referenced this pull request Jun 6, 2021
KAFKA-12892: Use dedicated root in ZK ACL test (apache#10821)
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

Comments