Skip to content

Conversation

@gharris1727
Copy link
Contributor

These tests leak sockets via Kafka clients, either directly or via KafkaStreams instances. The tests should close these resources when appropriate.

Committer Checklist (excluded from commit message)

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

Signed-off-by: Greg Harris <greg.harris@aiven.io>
Copy link
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.

Thanks for the PR! Very nice to get these leaks fixed.

})
);

// do not use the harness streams
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what this comment means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this, let me know if the new comment makes more sense or if you'd like it removed.

I was indicating that the KafkaStreams that the test harness set up for the test won't be used, and a different KafkaStreams was being substituted.

Signed-off-by: Greg Harris <greg.harris@aiven.io>
@gharris1727 gharris1727 requested a review from mjsax November 20, 2023 19:32
Copy link
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.

Thanks. LGTM.

@gharris1727
Copy link
Contributor Author

The streams test failures appear unrelated, as they are not subclasses of the changed tests, and all streams tests pass locally.

@gharris1727 gharris1727 merged commit 9f896ed into apache:trunk Nov 29, 2023
yyu1993 pushed a commit to yyu1993/kafka that referenced this pull request Feb 15, 2024
Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Matthias J. Sax <mjsax@apache.org>
AnatolyPopov pushed a commit to aiven/kafka that referenced this pull request Feb 16, 2024
Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Matthias J. Sax <mjsax@apache.org>
clolov pushed a commit to clolov/kafka that referenced this pull request Apr 5, 2024
Signed-off-by: Greg Harris <greg.harris@aiven.io>
Reviewers: Matthias J. Sax <mjsax@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants