Skip to content
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

[improve][test] broker: remove prometheus tests from flaky group #18554

Merged

Conversation

pgier
Copy link
Contributor

@pgier pgier commented Nov 20, 2022

The broker PromtheusMetricsTest should be reliable enough to run in the main broker test group. This will help prevent future merges from accidentally breaking the broker metrics.

Signed-off-by: Paul Gier paul.gier@datastax.com

Motivation

This just moves some tests out of the flaky group and into the broker group. This will help ensure that these tests pass before any new code changes.

Modifications

Changed the test group from flaky to broker for the prometheus metrics test.

Verifying this change

  • Make sure that the change passes the CI checks.

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
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: pgier#4

The broker PromtheusMetricsTest should be reliable enough to run in the main
broker test group.  This will help prevent future merges from accidentally
breaking the broker metrics.

Signed-off-by: Paul Gier <paul.gier@datastax.com>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 20, 2022
@pgier pgier changed the title [improve][tests] broker: remove prometheus tests from flaky group [improve][test] broker: remove prometheus tests from flaky group Nov 21, 2022
@Technoboy- Technoboy- added this to the 2.12.0 milestone Nov 21, 2022
@Technoboy- Technoboy- closed this Nov 21, 2022
@Technoboy- Technoboy- reopened this Nov 21, 2022
@nodece nodece requested a review from tisonkun November 21, 2022 02:32
@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2022

Codecov Report

Merging #18554 (45c6ad5) into master (bc1e0cb) will increase coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18554      +/-   ##
============================================
+ Coverage     47.50%   47.92%   +0.41%     
+ Complexity    10505     7866    -2639     
============================================
  Files           698      463     -235     
  Lines         67984    51345   -16639     
  Branches       7272     5458    -1814     
============================================
- Hits          32297    24607    -7690     
+ Misses        32112    23907    -8205     
+ Partials       3575     2831     -744     
Flag Coverage Δ
unittests 47.92% <ø> (+0.41%) ⬆️

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

Impacted Files Coverage Δ
...sar/broker/stats/metrics/ManagedLedgerMetrics.java 23.88% <0.00%> (-76.12%) ⬇️
.../pulsar/broker/service/SharedConsumerAssignor.java 3.70% <0.00%> (-74.08%) ⬇️
...rvice/schema/KeyValueSchemaCompatibilityCheck.java 21.62% <0.00%> (-45.95%) ⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java 0.00% <0.00%> (-40.75%) ⬇️
...oker/loadbalance/LoadResourceQuotaUpdaterTask.java 44.44% <0.00%> (-33.34%) ⬇️
...ar/broker/loadbalance/impl/BundleSplitterTask.java 60.00% <0.00%> (-20.00%) ⬇️
.../org/apache/pulsar/broker/lookup/LookupResult.java 56.66% <0.00%> (-20.00%) ⬇️
...ulsar/broker/namespace/NamespaceEphemeralData.java 68.18% <0.00%> (-18.19%) ⬇️
...e/pulsar/broker/stats/metrics/AbstractMetrics.java 36.48% <0.00%> (-17.57%) ⬇️
.../apache/pulsar/broker/admin/impl/PackagesBase.java 54.12% <0.00%> (-13.77%) ⬇️
... and 276 more

@tisonkun
Copy link
Member

Merging...

Thanks for taking care of tests @pgier!

@tisonkun tisonkun merged commit 738f186 into apache:master Nov 21, 2022
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Dec 9, 2022
Signed-off-by: Paul Gier <paul.gier@datastax.com>
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
Signed-off-by: Paul Gier <paul.gier@datastax.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants