-
Notifications
You must be signed in to change notification settings - Fork 14k
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
KAFKA-4646: Improve test coverage AbstractProcessorContext #2447
Conversation
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
public static Properties minimalStreamsConfig() { | ||
final Properties properties = new Properties(); | ||
properties.put(StreamsConfig.APPLICATION_ID_CONFIG, UUID.randomUUID().toString()); | ||
properties.put(StreamsConfig.BOOTSTRAP_SERVERS_CONFIG, "blah:9092"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: can we change blah
to anyServer
or something less blah-y? Otherwise LGTM
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
@Test(expected = IllegalStateException.class) | ||
public void shouldThrowIllegalStateExceptionOnRegisterWhenContextIsInitialized() throws Exception { | ||
context.initialized(); | ||
context.register(stateStore, false, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use try-catch
instead of expected
annotation -- not a single line test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-)
@Test(expected = IllegalStateException.class) | ||
public void shouldThrowIllegalStateExceptionOnTopicIfNoRecordContext() throws Exception { | ||
context.setRecordContext(null); | ||
context.topic(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
@Test(expected = IllegalStateException.class) | ||
public void shouldThrowIllegalStateExceptionOnPartitionIfNoRecordContext() throws Exception { | ||
context.setRecordContext(null); | ||
context.partition(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above.
@Test(expected = IllegalStateException.class) | ||
public void shouldThrowIllegalStateExceptionOnOffsetIfNoRecordContext() throws Exception { | ||
context.setRecordContext(null); | ||
context.offset(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
@Test(expected = IllegalStateException.class) | ||
public void shouldThrowIllegalStateExceptionOnTimestampIfNoRecordContext() throws Exception { | ||
context.setRecordContext(null); | ||
context.timestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and merged to trunk.
Exception paths in `register()`, `topic()`, `partition()`, `offset()`, and `timestamp()`, were not covered by any existing tests Author: Damian Guy <damian.guy@gmail.com> Reviewers: Eno Thereska, Matthias J. Sax Closes apache#2447 from dguy/KAFKA-4646
Exception paths in
register()
,topic()
,partition()
,offset()
, andtimestamp()
, were not covered by any existing tests