Skip to content

MINOR: Cleanup Core Module- Scala Modules (4/n) #19805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 6, 2025

Conversation

sjhajharia
Copy link
Contributor

@sjhajharia sjhajharia commented May 24, 2025

Now that Kafka Brokers support Java 17, this PR makes some changes in
core module. The changes in this PR are limited to only some Scala files
in the Core module's tests. The changes mostly include:

  • Collections.emptyList(), Collections.singletonList() and
    Arrays.asList() are replaced with List.of()
  • Collections.emptyMap() and Collections.singletonMap() are replaced
    with Map.of()
  • Collections.singleton() is replaced with Set.of()

To be clear, the directories being targeted in this PR from unit.kafka
module:

  • log
  • network
  • security
  • tools
  • utils

Reviewers: TengYao Chi [email protected]

Unverified

This user has not yet uploaded their public signing key.
@github-actions github-actions bot added triage PRs from the community core Kafka Broker tests Test fixes (including flaky tests) labels May 24, 2025
@@ -567,7 +567,7 @@ class AuthorizerTest extends QuorumTestHarness with BaseAuthorizerTest {
val acl3 = new AclBinding(resource2, new AccessControlEntry(principal.toString, WILDCARD_HOST, DESCRIBE, ALLOW))
val acl4 = new AclBinding(prefixedResource, new AccessControlEntry(wildcardPrincipal.toString, WILDCARD_HOST, READ, ALLOW))

authorizer1.createAcls(requestContext, List(acl1, acl2, acl3, acl4).asJava)
authorizer1.createAcls(requestContext, util.List.of(acl1, acl2, acl3, acl4))
assertEquals(Set(acl1, acl2, acl3, acl4), authorizer1.acls(AclBindingFilter.ANY).asScala.toSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change it to java.util.Set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving it to Set comes with one major issue -> The ordering.
Since its a assertEquals, if the ordering is not preserved, the test will turn flaky. And I somehow am not convinced to add a load amount of lines to use HashSet.

Unverified

This user has not yet uploaded their public signing key.
@sjhajharia
Copy link
Contributor Author

Hey @frankvicky
Thanks for the initial look. I have addressed most of the comments. Answered one.

@github-actions github-actions bot removed the triage PRs from the community label May 30, 2025
@sjhajharia
Copy link
Contributor Author

Gentle reminder @frankvicky

@frankvicky frankvicky merged commit a090dc3 into apache:trunk Jun 6, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved core Kafka Broker tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants