Skip to content

HOTFIX: fix RocksDBMetricsTest#9935

Merged
chia7712 merged 2 commits intoapache:trunkfrom
chia7712:MINOR-9935
Jan 20, 2021
Merged

HOTFIX: fix RocksDBMetricsTest#9935
chia7712 merged 2 commits intoapache:trunkfrom
chia7712:MINOR-9935

Conversation

@chia7712
Copy link
Copy Markdown
Member

@chia7712 chia7712 commented Jan 20, 2021

related to add160d

The description is changed so mock can't match the expected value.

Committer Checklist (excluded from commit message)

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

@chia7712
Copy link
Copy Markdown
Member Author

chia7712 commented Jan 20, 2021

@cadonna Could you take a look?

@showuon
Copy link
Copy Markdown
Member

showuon commented Jan 20, 2021

@chia7712 , thanks for the patch! But the test results failed other tests in RocksDBMetricsTest. FYI

@showuon showuon mentioned this pull request Jan 20, 2021
Merged
3 tasks
@chia7712 chia7712 changed the title HOTFIX: fix RocksDBMetricsTest#shouldAddEstimateNumKeysMetric HOTFIX: fix RocksDBMetricsTest Jan 20, 2021
@chia7712
Copy link
Copy Markdown
Member Author

chia7712 commented Jan 20, 2021

@showuon thanks for reminder. will fix them.

@cadonna
Copy link
Copy Markdown
Member

cadonna commented Jan 20, 2021

@chia7712 Thank you very much for the PR! Let me run the tests locally to double check.

@cadonna
Copy link
Copy Markdown
Member

cadonna commented Jan 20, 2021

@chia7712 I see a lot of

[ant:checkstyle] [ERROR] /Users/bruno/Code/apache/kafka/streams/src/test/java/org/apache/kafka/streams/integration/EmptyAssignmentTest.java:20:8: Unused import - org.apache.kafka.common.Metric. [UnusedImports]

When I try to run the test locally.

@cadonna
Copy link
Copy Markdown
Member

cadonna commented Jan 20, 2021 •
edited
Loading

@chia7712 Sorry my fault. I had a untracked file locally.

ijuma
ijuma approved these changes Jan 20, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Failures are unrelated, LGTM.

@ijuma ijuma mentioned this pull request Jan 20, 2021
Merged
3 tasks
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 20, 2021

@chia7712 Looks like the original PR was cherry-picked to 2.7 too, so let's do the same for this.

@chia7712 chia7712 merged commit 38b320a into apache:trunk Jan 20, 2021
@chia7712
Copy link
Copy Markdown
Member Author

chia7712 commented Jan 20, 2021

merge to trunk and 2.7

chia7712 added a commit that referenced this pull request Jan 20, 2021
@chia7712
HOTFIX: fix RocksDBMetricsTest (#9935)
8c221a4 
Reviewers: Ismael Juma <ismael@juma.me.uk>
@cadonna
Copy link
Copy Markdown
Member

cadonna commented Jan 20, 2021

Thank you!

@ableegoldman
Copy link
Copy Markdown
Member

ableegoldman commented Jan 20, 2021

Thanks for the fix @chia7712

@chia7712 chia7712 deleted the MINOR-9935 branch March 25, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ijuma ijuma ijuma approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chia7712 @showuon @cadonna @ijuma @ableegoldman