kafka icon indicating copy to clipboard operation
kafka copied to clipboard

KAFKA-14589 [3/3] Tests of ConsoleGroupCommand rewritten in java

Open nizhikov opened this issue 1 year ago • 3 comments

This PR is part of https://github.com/apache/kafka/pull/14471 Is contains some of ConsoleGroupCommand tests rewritten in java. Intention of separate PR is to reduce changes and simplify review.

Committer Checklist (excluded from commit message)

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

nizhikov avatar Feb 13 '24 18:02 nizhikov

Hello @showuon

Can you, please, take a look? I've checked CI results and it seems OK for me.

nizhikov avatar Feb 14 '24 15:02 nizhikov

Hello @mimaison @jolshan Can you, please, take a look?

nizhikov avatar Feb 15 '24 10:02 nizhikov

Hello @showuon @jolshan @mimaison

Can you, please, take a look?

nizhikov avatar Feb 26 '24 07:02 nizhikov

Hello, @rreddy-22 @dajac

Can you, please, take a look at these changes? It PR moves test of ConsoleGroupCommand from Authorizer test and Sasl test to tools. It very small (+200, -100).

Big PR where all command code rewritten in java - https://github.com/apache/kafka/pull/14471

Right now, changes in ConsoleGroupCommand leads to conflicts in my PR which is hard to resolve.

nizhikov avatar Feb 29 '24 12:02 nizhikov

Hello @chia7712

I checked CI results and failures seems unrelated to changes for me.

nizhikov avatar Mar 05 '24 07:03 nizhikov

@nizhikov I feel this PR is ready, and so please check (or list) the failed tests. If they are unconnected to this PR, I will merge it.

chia7712 avatar Mar 05 '24 21:03 chia7712

the failed tests pass on my machine.

./gradlew cleanTest core:test --tests LogDirFailureTest --tests ReplicaManagerTest --tests TransactionsTest --tests SaslPlaintextConsumerTest tools:test --tests MetadataQuorumCommandTest

will merge it

chia7712 avatar Mar 06 '24 09:03 chia7712

@nizhikov thanks for this refactor. let us deal with next PR :)

chia7712 avatar Mar 06 '24 09:03 chia7712

@chia7712 Thank you for the review and merge.

nizhikov avatar Mar 06 '24 09:03 nizhikov