KAFKA-20329: Test headers dsl.store.format further (3/N)#21820
KAFKA-20329: Test headers dsl.store.format further (3/N)#21820aliehsaeedii wants to merge 4 commits intoapache:trunkfrom
Conversation
github-actions
bot
added
triage
mjsax
added
kip
| if (withHeaders) { | ||
| streamsConfiguration.put(StreamsConfig.DSL_STORE_FORMAT_CONFIG, StreamsConfig.DSL_STORE_FORMAT_HEADERS); | ||
| } |
There was a problem hiding this comment.
Should we add a withHeader parameter to startStreams() for reducing the repeat code ?
frankvicky
left a comment
frankvicky
left a comment
There was a problem hiding this comment.
Sorry, I don't express clear.
There're tons of if conditions for headers config in test files.
If we repeat it several times, we should add a helper method to reduce boilerplate code.
| } | ||
|
|
||
| private void configureStreams(final boolean streamsProtocolEnabled, final String appID) { | ||
| private void configureStreams(final boolean withHeaders, final String appID) { |
There was a problem hiding this comment.
Why do we remove streamsProtocolEnabled ?
| @ParameterizedTest | ||
| @ValueSource(booleans = {true, false}) | ||
| public void shouldAddMetricsOnAllLevels(final boolean streamsProtocolEnabled) throws Exception { | ||
| if (streamsProtocolEnabled) { |
| ); | ||
| createStateForRestoration(inputStream, 0); | ||
|
|
||
| final boolean useNewProtocol = false; |
There was a problem hiding this comment.
Cannot follow -- also, if we don't want to test "streams" protocol any longer, why do we needs this constant at all?
|
|
||
| sendEvents(inputTopic, sampleData); | ||
|
|
||
| final boolean useNewProtocol = false; |
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@aliehsaeedii thanks for enhancing the tests!
| mkEntry(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory().getPath()) | ||
| )); | ||
| if (withHeaders) { | ||
| props.put(StreamsConfig.DSL_STORE_FORMAT_CONFIG, StreamsConfig.DSL_STORE_FORMAT_HEADERS); |
There was a problem hiding this comment.
The helper method maybeSetDslStoreFormatHeaders is so beautiful. We should let other classes be obsessed with it too.
| mkEntry(StreamsConfig.STATE_DIR_CONFIG, TestUtils.tempDirectory().getPath()) | ||
| )); | ||
| if (withHeaders) { | ||
| props.put(StreamsConfig.DSL_STORE_FORMAT_CONFIG, StreamsConfig.DSL_STORE_FORMAT_HEADERS); |
|
retrigger CI |
4 participants
This PR parametrizes 10 integration test classes (49 test methods total)
to ensure comprehensive testing coverage for both default and
header-based DSL store formats. Each test now runs twice: once with
the default store format and once with dsl.store.format=headers.
Reviewers: TengYao Chi frankvicky@apache.org