Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[fix] [test] fix testGetMetrics in ExtensibleLoadManagerImplTest. #22864

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

thetumbled
Copy link
Member

@thetumbled thetumbled commented Jun 7, 2024

Fixes #22871

Motivation

As #22820 set the default value of LoadBalancerMemoryResourceWeight to 0, some behavior changes resulting into the failure of org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImplTest#testGetMetrics.

Modifications

Set the value of LoadBalancerMemoryResourceWeight to 1 to pass the test.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@thetumbled
Copy link
Member Author

PTAL, thanks. @heesung-sn @Demogorgon314

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 7, 2024
@thetumbled
Copy link
Member Author

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.26%. Comparing base (bbc6224) to head (0e13c30).
Report is 355 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22864      +/-   ##
============================================
- Coverage     73.57%   73.26%   -0.31%     
+ Complexity    32624     2511   -30113     
============================================
  Files          1877     1889      +12     
  Lines        139502   141830    +2328     
  Branches      15299    15563     +264     
============================================
+ Hits         102638   103916    +1278     
- Misses        28908    29887     +979     
- Partials       7956     8027      +71     
Flag Coverage Δ
inttests 27.24% <ø> (+2.65%) ⬆️
systests 24.85% <ø> (+0.53%) ⬆️
unittests 72.28% <ø> (-0.56%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 381 files with indirect coverage changes

@BewareMyPower BewareMyPower added this to the 3.4.0 milestone Jun 7, 2024
@BewareMyPower
Copy link
Contributor

I'm wondering why this failed test was not detected before?

@thetumbled
Copy link
Member Author

I'm wondering why this failed test was not detected before?

This is a flaky test, which can be ignored when merging.

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing.

@lhotari lhotari merged commit 6f1f7ba into apache:master Jun 7, 2024
54 of 55 checks passed
Copy link
Contributor

@heesung-sn heesung-sn left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky-test: ExtensibleLoadManagerImplTest.testGetMetrics (fails consistently)
6 participants