KAFKA-17574: Allow overriding TestKitNodes baseDirectory #17225
Conversation
| cluster.nodes().brokerNodes().values().forEach(broker -> { | ||
| assertTrue(Paths.get(broker.metadataDirectory()).startsWith(baseDirectory)); | ||
| }); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
it is find to throw exception directly so could you please remove the catch?
There was a problem hiding this comment.
Sure, this was copied from the test above. Would you prefer I also remove it there?
There was a problem hiding this comment.
yes, please do a bit refactor :)
There was a problem hiding this comment.
👍 Comments should have been addressed in the latest commit
| setCombined(true). | ||
| setNumControllerNodes(1).build()).build()) { | ||
| assertEquals(cluster.nodes().baseDirectory(), baseDirectory.toFile().getAbsolutePath()); | ||
| cluster.nodes().controllerNodes().values().forEach(controller -> { |
| cluster.nodes().controllerNodes().values().forEach(controller -> { | ||
| assertTrue(Paths.get(controller.metadataDirectory()).startsWith(baseDirectory)); | ||
| }); | ||
| cluster.nodes().brokerNodes().values().forEach(broker -> { |
|
@srdo Could you please sync trunk? |
|
@chia7712 Sure, squashed and rebased to latest. |
|
@srdo sorry that we have a big refactor for the code. Could you please fix the conflicts again? |
|
@chia7712 Sure, done |
|
@srdo sorry that could you please fix the conflicts again? |
srdo
force-pushed
the
kafka-17574
branch
2 times, most recently
from
06e5ee0 to
d38ad47
Compare
|
@chia7712 done |
|
@srdo could you please rebase code to trigger CI again? |
|
I feel the failed tests are unrelated. |
This allows shutting down a KafkaClusterTestKit from a JVM shutdown hook without risking error logs because the base directory has already been deleted by the shutdown hook TestUtils.tempDirectory sets up.
|
@chia7712 Sure, done. FWIW ShareConsumerTest passes for me locally. |
This allows shutting down a KafkaClusterTestKit from a JVM shutdown hook without risking error logs because the base directory has already been deleted by the shutdown hook TestUtils.tempDirectory sets up. Reviewers: Chia-Ping Tsai <[email protected]>
This allows shutting down a KafkaClusterTestKit from a JVM shutdown hook without risking error logs because the base directory has already been deleted by the shutdown hook TestUtils.tempDirectory sets up. Reviewers: Chia-Ping Tsai <[email protected]>
This allows shutting down a KafkaClusterTestKit from a JVM shutdown hook without risking error logs because the base directory has already been deleted by the shutdown hook TestUtils.tempDirectory sets up. Reviewers: Chia-Ping Tsai <[email protected]>
This allows shutting down a KafkaClusterTestKit from a JVM shutdown hook without risking error logs because the base directory has already been deleted by the shutdown hook TestUtils.tempDirectory sets up.
Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.
Added a test that uses the new method and asserts that the controller and broker metadata directories are inside the specified directory.
Committer Checklist (excluded from commit message)