Skip to content

MINOR: convert some more junit tests to support KRaft#12456

Merged
cmccabe merged 2 commits intoapache:trunkfrom
cmccabe:2022-07-28-junit-tests
Jul 29, 2022
Merged

MINOR: convert some more junit tests to support KRaft#12456
cmccabe merged 2 commits intoapache:trunkfrom
cmccabe:2022-07-28-junit-tests

Conversation

@cmccabe
Copy link
Contributor

@cmccabe cmccabe commented Jul 28, 2022

Introduce TestUtils#waitUntilLeaderIsElectedOrChangedWithAdmin, a ZK-free alternative to
TestUtils#waitUntilLeaderIsElectedOrChanged.

Convert PlaintextProducerSendTest, SslProducerSendTest, TransactionsWithMaxInFlightOneTest,
AddPartitionsToTxnRequestServerTest and KafkaMetricsReporterTest to support KRaft

Copy link
Member

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

LGTM, just one question inline

Copy link
Member

Choose a reason for hiding this comment

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

Would it simplify any of this to use computeUntilTrue or one of the other helper "until" methods in TestUtils?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good idea for a refactor, but this was just copied from the ZK version. I didn't want to rewrite it too much since it's kind of complex.

I looked at this again and felt a bit bad about duplicating so much code between the ZK and KRaft versions. So I revised it so that they both call into the same function, avoiding the duplication.

Copy link
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

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

Left one comment, others LGTM.

Copy link
Member

Choose a reason for hiding this comment

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

we can't convert BaseProducerSendTest now because tests inSslProducerSendTest are all failing

Copy link
Contributor Author

@cmccabe cmccabe Jul 29, 2022

Choose a reason for hiding this comment

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

Thanks for pointing this out. Looks like it was an issue with using the wrong endpoint in one of the topic creates. Fixed.

cmccabe added 2 commits July 29, 2022 13:23
Introduce TestUtils#waitUntilLeaderIsElectedOrChangedWithAdmin, a ZK-free alternative to
TestUtils#waitUntilLeaderIsElectedOrChanged.

Convert PlaintextProducerSendTest, SslProducerSendTest, TransactionsWithMaxInFlightOneTest,
AddPartitionsToTxnRequestServerTest and KafkaMetricsReporterTest to support KRaft
@cmccabe cmccabe force-pushed the 2022-07-28-junit-tests branch 2 times, most recently from d31f4bb to be721cb Compare July 29, 2022 20:23
@cmccabe cmccabe merged commit e26772e into apache:trunk Jul 29, 2022
@cmccabe cmccabe deleted the 2022-07-28-junit-tests branch July 29, 2022 20:36
ijuma added a commit to confluentinc/kafka that referenced this pull request Aug 5, 2022
…(5 August 2022)

Version related conflicts:
* Jenkinsfile
* gradle.properties
* streams/quickstart/java/pom.xml
* streams/quickstart/java/src/main/resources/archetype-resources/pom.xml
* streams/quickstart/pom.xml
* tests/kafkatest/__init__.py
* tests/kafkatest/version.py

* commit 'add7cd85baa61cd0e1430': (66 commits)
KAFKA-14136 Generate ConfigRecord for brokers even if the value is
unchanged (apache#12483)
  HOTFIX / KAFKA-14130: Reduce RackAwarenesssTest to unit Test (apache#12476)
  MINOR: Remove ARM/PowerPC builds from Jenkinsfile (apache#12380)
  KAFKA-14111 Fix sensitive dynamic broker configs in KRaft (apache#12455)
  KAFKA-13877: Fix flakiness in RackAwarenessIntegrationTest (apache#12468)
KAFKA-14129: KRaft must check manual assignments for createTopics are
contiguous (apache#12467)
KAFKA-13546: Do not fail connector validation if default topic
creation group is explicitly specified (apache#11615)
KAFKA-14122: Fix flaky test
DynamicBrokerReconfigurationTest#testKeyStoreAlter (apache#12452)
  MINOR; Use right enum value for broker registration change (apache#12236)
  MINOR; Synchronize access to snapshots' TreeMap (apache#12464)
  MINOR; Bump trunk to 3.4.0-SNAPSHOT (apache#12463)
  MINOR: Stop logging 404s at ERROR level in Connect
KAFKA-14095: Improve handling of sync offset failures in MirrorMaker
(apache#12432)
  Minor: enable index for emit final sliding window (apache#12461)
  MINOR: convert some more junit tests to support KRaft (apache#12456)
  KAFKA-14108: Ensure both JUnit 4 and JUnit 5 tests run (apache#12441)
  MINOR: Remove code of removed metric (apache#12453)
MINOR: Update comment on verifyTaskGenerationAndOwnership method in
DistributedHerder
KAFKA-14012: Add warning to closeQuietly documentation about method
references of null objects (apache#12321)
  MINOR: Fix static mock usage in ThreadMetricsTest (apache#12454)
  ...
cmccabe added a commit that referenced this pull request Aug 9, 2022
* MINOR: convert some more junit tests to support KRaft

Introduce TestUtils#waitUntilLeaderIsElectedOrChangedWithAdmin, a ZK-free alternative to
TestUtils#waitUntilLeaderIsElectedOrChanged.

Convert PlaintextProducerSendTest, SslProducerSendTest, TransactionsWithMaxInFlightOneTest,
AddPartitionsToTxnRequestServerTest and KafkaMetricsReporterTest to support KRaft

Reviewers: dengziming <dengziming1993@gmail.com>, David Arthur <mumrah@gmail.com>
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