Skip to content
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

MINOR: Fix remaining core, connect and clients tests to pass with Java 11 #5771

Merged
merged 4 commits into from
Oct 10, 2018

Conversation

ijuma
Copy link
Contributor

@ijuma ijuma commented Oct 10, 2018

  • SslFactoryTest should use SslFactory to create SSLEngine
  • Use Mockito instead of EasyMock in ConsoleConsumerTest as one of
    the tests mocks a standard library class and the latest released EasyMock
    version can't do that when Java 11 is used.
  • Avoid mocking ConcurrentMap in SourceTaskOffsetCommitterTest
    for similar reasons. As it happens, mocking is not actually needed here.

Committer Checklist (excluded from commit message)

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

- SslFactoryTest should use SslFactory to create SSLEngine
- Use Mockito instead of EasyMock in ConsoleConsumerTest as one of
the tests mocks a standard library class and EasyMock can't do that
when Java 11 is used.

SourceTaskOffsetCommitterTest in connect-runtime is the only test
that still fails with Java 11.
@ijuma ijuma changed the title MINOR: Fix remaining core and clients tests when running with Java 11 MINOR: Fix remaining core, connect and clients tests to pass with Java 11 Oct 10, 2018
Copy link
Contributor

@rajinisivaram rajinisivaram left a comment

Choose a reason for hiding this comment

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

@ijuma Thanks for the PR. Left a minor comment, apart from that LGTM.

PowerMock.replayAll();

committer.schedule(taskId, task);
assertTrue(taskWrapper.hasCaptured());
assertNotNull(taskWrapper.getValue());
assertEquals(Utils.mkMap(mkEntry(taskId, commitFuture)), committers);
Copy link
Contributor

Choose a reason for hiding this comment

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

mkMap and mkEntry are both statics in Utils, why do we have static import for one and not the other? Anyway, perhaps this could just be Collections.singletonMap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, changed to use singletonMap. Also removed some imports and an unnecessary @PrepareForTest.

@ijuma
Copy link
Contributor Author

ijuma commented Oct 10, 2018

One job passed and the other had an unrelated test failure: testTrustStoreAlter. Merging to trunk and 2.1.

@ijuma ijuma merged commit adb3a95 into apache:trunk Oct 10, 2018
@ijuma ijuma deleted the java-11-test-fixes branch October 10, 2018 20:31
ijuma added a commit that referenced this pull request Oct 10, 2018
…a 11 (#5771)

- SslFactoryTest should use SslFactory to create SSLEngine
- Use Mockito instead of EasyMock in `ConsoleConsumerTest` as one of
the tests mocks a standard library class and the latest released EasyMock
version can't do that when Java 11 is used.
- Avoid mocking `ConcurrentMap` in `SourceTaskOffsetCommitterTest`
for similar reasons. As it happens, mocking is not actually needed here.

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
…a 11 (apache#5771)

- SslFactoryTest should use SslFactory to create SSLEngine
- Use Mockito instead of EasyMock in `ConsoleConsumerTest` as one of
the tests mocks a standard library class and the latest released EasyMock
version can't do that when Java 11 is used.
- Avoid mocking `ConcurrentMap` in `SourceTaskOffsetCommitterTest`
for similar reasons. As it happens, mocking is not actually needed here.

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.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
2 participants