xiangfu0 opened a new pull request, #18259: URL: https://github.com/apache/pinot/pull/18259
## Summary Moves Yammer/Dropwizard-specific metrics tests from `pinot-common` test sources into their respective plugin modules (`pinot-yammer`, `pinot-dropwizard`), removing the cross-directional test dependency that had `pinot-common` depending on plugin modules for its own tests. ## Why `pinot-common/pom.xml` previously pulled in `pinot-yammer` and `pinot-dropwizard` as `<scope>test</scope>` dependencies, solely because a handful of test classes under `pinot-common/src/test/.../metrics/` directly exercised the Yammer/Dropwizard implementations. This is backwards — plugin modules should depend on `pinot-common`, not the other way around — and it blocked follow-up work (e.g. making `pinot-common` self-contained for Java 21 bytecode decisions). ## What changed - **Move Yammer-specific tests** from `pinot-common/src/test/.../metrics/prometheus/yammer/` → `pinot-plugins/pinot-metrics/pinot-yammer/src/test/.../prometheus/`. Includes `AbstractMetricsTest`, `MetricsInspector`, `PinotMetricUtilsTest`, and the four `Yammer*PrometheusMetricsTest` classes. - **Move Dropwizard-specific tests** from `pinot-common/src/test/.../metrics/prometheus/dropwizard/` → `pinot-plugins/pinot-metrics/pinot-dropwizard/src/test/.../prometheus/`. (These were `@Test(enabled = false)` but the files are preserved for future re-enablement.) - **Rename** `pinot-yammer`'s version of `MetricValueUtils` → `YammerMetricValueUtils` to avoid FQN clash with the one that remains in `pinot-common` for non-yammer-coupled uses. - **Refactor `ConnectionFailureDetectorTest`** to construct `YammerMetricsFactory` directly instead of relying on `ServiceLoader`, so it no longer requires the yammer plugin on the test classpath for service discovery. - **Fix `AbstractMetrics.getGaugeValue()`** to fall back to registry lookup when the gauge was registered via `setOrUpdateGauge(name, Supplier)` and thus isn't present in `_gaugeValues`. Surfaced by the moved tests; covers all `PinotGauge` subtypes. - **Drop `pinot-yammer` and `pinot-dropwizard` test dependencies** from `pinot-common/pom.xml`. - **Add `pinot-common` test dependency** to `pinot-yammer` and `pinot-dropwizard` poms (direction that actually makes sense). ## Impact - `pinot-common` no longer has circular-feeling test deps on plugin modules. - Plugin-specific test coverage now lives with the plugin implementation, which is the standard layout. - No production behavior change except the `getGaugeValue()` fallback fix, which corrects a pre-existing bug where supplier-registered gauges returned missing values. ## Test plan - [x] `./mvnw -pl pinot-common,pinot-plugins/pinot-metrics/pinot-yammer,pinot-plugins/pinot-metrics/pinot-dropwizard -am test` passes locally - [x] All previously-passing Yammer prometheus metrics tests still pass in their new location - [x] `ConnectionFailureDetectorTest` still passes with the direct `YammerMetricsFactory` construction - [x] `AbstractMetrics.getGaugeValue()` returns the correct value for supplier-registered gauges -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
