Skip to content

MINOR: Streams upgrade system test cleanup#7571

Merged
bbejeck merged 4 commits intoapache:trunkfrom
bbejeck:MINOR_clean_up_streams_upgrade_system_test
Oct 24, 2019
Merged

MINOR: Streams upgrade system test cleanup#7571
bbejeck merged 4 commits intoapache:trunkfrom
bbejeck:MINOR_clean_up_streams_upgrade_system_test

Conversation

@bbejeck
Copy link
Copy Markdown
Member

@bbejeck bbejeck commented Oct 21, 2019

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

@bbejeck bbejeck added streams tests Test fixes (including flaky tests) labels Oct 21, 2019
@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Oct 21, 2019

This is a follow-up PR to perform the same clean-up from #7552

Ran all system tests http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2019-10-21--001.1571679740--bbejeck--MINOR_clean_up_streams_upgrade_system_test--8e61da4/report.html

All tests ran. Note there is one failure, but it' unrelated to these changes. I'm currently investigating the failure.

ping @guozhangwang @mjsax

@guozhangwang
Copy link
Copy Markdown
Contributor

The change lgtm modulo the test failure, ping @mjsax again.

@bbejeck bbejeck force-pushed the MINOR_clean_up_streams_upgrade_system_test branch from 52a8464 to e5f5697 Compare October 23, 2019 19:35
@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Oct 23, 2019

Turns out the test failure was related to logging changes in #7537 and I just needed to update the expected log statements. I ran a new branch builder for the failing test

http://confluent-kafka-branch-builder-system-test-results.s3-us-west-2.amazonaws.com/2019-10-23--001.1571861801--bbejeck--MINOR_clean_up_streams_upgrade_system_test--e5f5697/report.html

The test is showing some flakiness, but it's unrelated to the changes in this PR and should be pursued in a separate follow-on effort.

@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Oct 23, 2019

ping @guozhangwang and @mjsax updated this

System.err.println("StreamsUpgradeTest requires two argument (kafka-url, properties-file) but only " + args.length + " provided: "
+ (args.length > 0 ? args[0] : ""));
if (args.length < 1) {
System.err.println("StreamsUpgradeTest requires one argument (properties-file) but provided none");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do we remove args in print?

if (args.length < 2) {
System.err.println("StreamsUpgradeTest requires two argument (kafka-url, properties-file) but only " + args.length + " provided: "
+ (args.length > 0 ? args[0] : ""));
if (args.length < 1) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Will we hit NPE in this case?

@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Oct 24, 2019

@guozhangwang
Copy link
Copy Markdown
Contributor

LGTM! Please feel free to merge @bbejeck

@bbejeck bbejeck merged commit c015169 into apache:trunk Oct 24, 2019
@bbejeck
Copy link
Copy Markdown
Member Author

bbejeck commented Oct 24, 2019

Merged #7571 into trunk

@bbejeck bbejeck deleted the MINOR_clean_up_streams_upgrade_system_test branch October 24, 2019 14:28
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.

3 participants