Copilot commented on code in PR #18947:
URL: https://github.com/apache/hudi/pull/18947#discussion_r3380228801
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieMetrics.java:
##########
@@ -322,6 +327,317 @@ public MockHoodieActiveTimeline(HoodieInstant...
instants) {
}
}
+ // -----------------------------------------------------------------------
+ // Metrics-off safety tests (NPE guards)
+ // -----------------------------------------------------------------------
+
+ private HoodieMetrics buildMetricsOff() {
+ HoodieWriteConfig offConfig = mock(HoodieWriteConfig.class);
+ when(offConfig.isMetricsOn()).thenReturn(false);
+ when(offConfig.getTableName()).thenReturn("test_table");
+ return new HoodieMetrics(offConfig, HoodieTestUtils.getDefaultStorage());
+ }
+
+ @Test
+ public void testTimerContextsReturnNullWhenMetricsOff() {
+ HoodieMetrics metricsOff = buildMetricsOff();
+ assertNull(metricsOff.getRollbackCtx());
+ assertNull(metricsOff.getCompactionCtx());
+ assertNull(metricsOff.getLogCompactionCtx());
+ assertNull(metricsOff.getClusteringCtx());
+ assertNull(metricsOff.getCleanCtx());
+ assertNull(metricsOff.getArchiveCtx());
+ assertNull(metricsOff.getCommitCtx());
+ assertNull(metricsOff.getFinalizeCtx());
+ assertNull(metricsOff.getDeltaCommitCtx());
+ assertNull(metricsOff.getIndexCtx());
+ assertNull(metricsOff.getSourceReadAndIndexTimerCtx());
+ assertNull(metricsOff.getConflictResolutionCtx());
+ }
+
+ @Test
+ public void testUpdateMethodsAreNoOpsWhenMetricsOff() {
+ HoodieMetrics metricsOff = buildMetricsOff();
+ HoodieCommitMetadata metadata = mock(HoodieCommitMetadata.class);
+
+ assertDoesNotThrow(() -> metricsOff.updateCommitMetrics(0L, 0L, metadata,
"commit"));
+ assertDoesNotThrow(() -> metricsOff.updateRollbackMetrics(0L, 0L));
+ assertDoesNotThrow(() -> metricsOff.updateCleanMetrics(0L, 0));
+ assertDoesNotThrow(() -> metricsOff.updateFinalizeWriteMetrics(0L, 0L));
+ assertDoesNotThrow(() -> metricsOff.updateIndexMetrics("action", 0L));
+ assertDoesNotThrow(() ->
metricsOff.updateSourceReadAndIndexMetrics("action", 0L));
+ assertDoesNotThrow(() -> metricsOff.updateArchiveMetrics(0L, 0));
+ assertDoesNotThrow(() -> metricsOff.updateArchivalMetrics(new
HashMap<>()));
+ assertDoesNotThrow(() -> metricsOff.updatePostCommitMetrics(true, 0L));
+ assertDoesNotThrow(() -> metricsOff.updatePostCommitMetrics(false, 0L));
+ assertDoesNotThrow(() -> metricsOff.reportMetrics("action", "metric", 0L));
+ assertDoesNotThrow(() ->
metricsOff.updateClusteringFileCreationMetrics(0L));
+ assertDoesNotThrow(() -> metricsOff.emitRollbackFailure("SomeException"));
+ assertDoesNotThrow(() -> metricsOff.emitRollbackFailure(null));
+ assertDoesNotThrow(() -> metricsOff.emitCompactionRequested());
+ assertDoesNotThrow(() -> metricsOff.emitCompactionCompleted());
+ assertDoesNotThrow(() -> metricsOff.emitIndexTypeMetrics(0));
+ assertDoesNotThrow(() -> metricsOff.emitMetadataEnablementMetrics(false,
false, false, false));
+ assertDoesNotThrow(() -> metricsOff.emitVersionMetrics());
+ }
+
+ @Test
+ public void
testConflictResolutionMetricsAreNoOpsWhenMetricsOffButLockingEnabled() {
+ // Key NPE scenario: global metrics are off so metrics field is null.
+ // The isMetricsOn() && isLockingMetricsEnabled() guard short-circuits
before
+ // reaching getCounter() -> metrics.getRegistry(), so no NPE should occur.
+ HoodieWriteConfig offConfig = mock(HoodieWriteConfig.class);
+ when(offConfig.isMetricsOn()).thenReturn(false);
+ when(offConfig.getTableName()).thenReturn("test_table");
+ HoodieMetrics metricsOff = new HoodieMetrics(offConfig,
HoodieTestUtils.getDefaultStorage());
Review Comment:
`testConflictResolutionMetricsAreNoOpsWhenMetricsOffButLockingEnabled` does
not actually enable locking metrics on the mocked `HoodieWriteConfig`. With
Mockito, `isLockingMetricsEnabled()` will default to `false`, so this test will
pass even if the `isMetricsOn() && isLockingMetricsEnabled()` short-circuit
were removed, reducing the value of the regression test for the reported NPE
scenario.
--
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]
