Skip to content

Commit f2e7273

Browse files
KaiSernLimCopilot
andcommitted
fix: resolve CI failures from spotless, metric count, and branch coverage
- DeferredVersionSwapService.java: wrap long isAbandonedDeferredVersion return at the && operator and collapse split childVersion assignment to one line (spotlessJavaCheck violations) - TestDeferredVersionSwapServiceWithSequentialRollout.java: condense abandonedParentStatuses() array to match google-java-format style (spotlessJavaCheck violation) - ServerMetricEntityTest: update expected count 185 → 186 to reflect the new VERSION_COUNT entry added to StorageEngineOtelMetricEntity - AsyncMetricEntityStateOneEnumTest / AsyncMetricEntityStateTwoEnumsTest: add testCloseUnregistersObservableDoubleGauge and testCloseOnDisabledInstanceIsNoOp to cover the two missing branches in the new close() method (fixes venice-client-common diffCoverage < 50%) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 5e3d9cc commit f2e7273

5 files changed

Lines changed: 73 additions & 8 deletions

File tree

clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ServerMetricEntityTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
public class ServerMetricEntityTest {
2323
@Test
2424
public void testServerMetricEntitiesCount() {
25-
assertEquals(SERVER_METRIC_ENTITIES.size(), 185, "Expected 185 unique metric entities");
25+
assertEquals(SERVER_METRIC_ENTITIES.size(), 186, "Expected 186 unique metric entities");
2626
}
2727

2828
/**

internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverOneEnum;
2020
import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverOneEnum;
2121
import io.opentelemetry.api.common.Attributes;
22+
import io.opentelemetry.api.metrics.ObservableDoubleGauge;
2223
import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
2324
import io.opentelemetry.api.metrics.ObservableLongGauge;
2425
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
@@ -102,9 +103,40 @@ public void testCloseUnregistersObservableGauge() {
102103
verify(gauge).close();
103104
}
104105

106+
@Test
107+
public void testCloseUnregistersObservableDoubleGauge() {
108+
when(mockMetricEntity.getMetricType()).thenReturn(MetricType.ASYNC_DOUBLE_GAUGE);
109+
ObservableDoubleGauge gauge = mock(ObservableDoubleGauge.class);
110+
when(mockOtelRepository.registerObservableDoubleGauge(eq(mockMetricEntity), any())).thenReturn(gauge);
111+
AsyncMetricEntityStateOneEnum<DimensionEnum1> metricState = AsyncMetricEntityStateOneEnum.create(
112+
mockMetricEntity,
113+
mockOtelRepository,
114+
baseDimensionsMap,
115+
DimensionEnum1.class,
116+
e -> e,
117+
(state, e) -> 1L);
118+
119+
metricState.close();
120+
121+
verify(gauge).close();
122+
}
123+
124+
@Test
125+
public void testCloseOnDisabledInstanceIsNoOp() {
126+
AsyncMetricEntityStateOneEnum<DimensionEnum1> metricState = AsyncMetricEntityStateOneEnum.create(
127+
mockMetricEntity,
128+
null /* OTel disabled */,
129+
baseDimensionsMap,
130+
DimensionEnum1.class,
131+
e -> e,
132+
(state, e) -> 1L);
133+
134+
// Must not throw when instrument is null.
135+
metricState.close();
136+
}
137+
105138
@Test
106139
public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() {
107-
// liveStateResolver returns state for DIMENSION_ONE only; DIMENSION_TWO is dormant.
108140
LiveStateResolverOneEnum<DimensionEnum1, Object> liveStateResolver =
109141
e -> e == DimensionEnum1.DIMENSION_ONE ? "live" : null;
110142
ValueResolverOneEnum<Object, DimensionEnum1> valueResolver = (state, e) -> 42L;

internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverTwoEnums;
2121
import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverTwoEnums;
2222
import io.opentelemetry.api.common.Attributes;
23+
import io.opentelemetry.api.metrics.ObservableDoubleGauge;
2324
import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
2425
import io.opentelemetry.api.metrics.ObservableLongGauge;
2526
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
@@ -113,6 +114,40 @@ public void testCloseUnregistersObservableGauge() {
113114
verify(gauge).close();
114115
}
115116

117+
@Test
118+
public void testCloseUnregistersObservableDoubleGauge() {
119+
when(mockMetricEntity.getMetricType()).thenReturn(MetricType.ASYNC_DOUBLE_GAUGE);
120+
ObservableDoubleGauge gauge = mock(ObservableDoubleGauge.class);
121+
when(mockOtelRepository.registerObservableDoubleGauge(eq(mockMetricEntity), any())).thenReturn(gauge);
122+
AsyncMetricEntityStateTwoEnums<DimensionEnum1, DimensionEnum2> metricState = AsyncMetricEntityStateTwoEnums.create(
123+
mockMetricEntity,
124+
mockOtelRepository,
125+
baseDimensionsMap,
126+
DimensionEnum1.class,
127+
DimensionEnum2.class,
128+
(e1, e2) -> e1,
129+
(state, e1, e2) -> 1L);
130+
131+
metricState.close();
132+
133+
verify(gauge).close();
134+
}
135+
136+
@Test
137+
public void testCloseOnDisabledInstanceIsNoOp() {
138+
AsyncMetricEntityStateTwoEnums<DimensionEnum1, DimensionEnum2> metricState = AsyncMetricEntityStateTwoEnums.create(
139+
mockMetricEntity,
140+
null /* OTel disabled */,
141+
baseDimensionsMap,
142+
DimensionEnum1.class,
143+
DimensionEnum2.class,
144+
(e1, e2) -> e1,
145+
(state, e1, e2) -> 1L);
146+
147+
// Must not throw when instrument is null.
148+
metricState.close();
149+
}
150+
116151
@Test
117152
public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() {
118153
LiveStateResolverTwoEnums<DimensionEnum1, DimensionEnum2, Object> liveStateResolver = (e1, e2) -> {

services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,8 @@ private static String getVersionProcessingKey(String clusterName, String storeNa
971971
}
972972

973973
private static boolean isAbandonedDeferredVersion(Version version) {
974-
return version != null && version.isVersionSwapDeferred() && ABANDONED_VERSION_STATUSES.contains(version.getStatus());
974+
return version != null && version.isVersionSwapDeferred()
975+
&& ABANDONED_VERSION_STATUSES.contains(version.getStatus());
975976
}
976977

977978
private void submitTerminalVersionReconciliationTasks(
@@ -1393,8 +1394,7 @@ private boolean reconcileAbandonedVersionInChildRegions(
13931394
}
13941395

13951396
StoreInfo childStore = storeResponse.getStore();
1396-
Version childVersion =
1397-
getVersionFromStoreInRegion(region, storeName, targetVersionNum, storeResponse);
1397+
Version childVersion = getVersionFromStoreInRegion(region, storeName, targetVersionNum, storeResponse);
13981398
if (childVersion == null) {
13991399
allRegionsTerminal = false;
14001400
continue;

services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,9 +593,7 @@ public void testSequentialRolloutPostSwapValidationRollbackMarksNonTargetRegions
593593

594594
@DataProvider(name = "abandonedParentStatuses")
595595
public Object[][] abandonedParentStatuses() {
596-
return new Object[][] {
597-
{ VersionStatus.ERROR },
598-
{ VersionStatus.PARTIALLY_ONLINE },
596+
return new Object[][] { { VersionStatus.ERROR }, { VersionStatus.PARTIALLY_ONLINE },
599597
{ VersionStatus.ROLLED_BACK } };
600598
}
601599

0 commit comments

Comments
 (0)