From 52e7fa326de583405b7e4d7756234755f64f2a2f Mon Sep 17 00:00:00 2001 From: Joseph Grogan Date: Mon, 15 Jun 2026 13:08:47 -0400 Subject: [PATCH 1/3] Fix tests incorrectly using kubeconfig instead of mocking --- .../hoptimator/k8s/K8sJobDeployerTest.java | 29 +++----- .../logical/LogicalTableDriverTest.java | 66 +++++++++++-------- 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java b/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java index 023be9332..54da16c8d 100644 --- a/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java +++ b/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import java.sql.SQLException; @@ -24,18 +25,25 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Properties; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class K8sJobDeployerTest { + // K8sJobDeployer.specify() calls ConfigService.config(context.connection(), ...), which runs the + // real ServiceLoader and invokes K8sConfigProvider.loadConfig() -> K8sContext.create(connection). + // Without this static mock, create() would read the developer's real ~/.kube/config and block on + // an interactive cluster login, stalling the unit test. Stubbing it keeps the test hermetic. + @Mock + private MockedStatic contextStatic; + @Mock private HoptimatorConnection connection; @@ -60,6 +68,7 @@ K8sYamlApi createYamlApi(K8sContext context) { } }; when(mockContext.connection()).thenReturn(connection); + contextStatic.when(() -> K8sContext.create(any())).thenReturn(mockContext); } private Job createTestJob(Sink sink) { @@ -94,8 +103,6 @@ K8sSnapshot createSnapshot(K8sContext context) { @Test void specifyWithNoTemplatesReturnsEmpty() throws SQLException { - when(connection.connectionProperties()).thenReturn(new Properties()); - Sink sink = new Sink("sinkdb", Arrays.asList("schema", "sink_table"), Collections.emptyMap()); Job job = createTestJob(sink); @@ -110,8 +117,6 @@ void specifyWithNoTemplatesReturnsEmpty() throws SQLException { @Test void specifyRendersMatchingTemplate() throws SQLException { - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() @@ -132,8 +137,6 @@ void specifyRendersMatchingTemplate() throws SQLException { @Test void specifyFiltersOutNonMatchingDatabases() throws SQLException { - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() @@ -153,8 +156,6 @@ void specifyFiltersOutNonMatchingDatabases() throws SQLException { @Test void specifyWithNullDatabasesMatchesAll() throws SQLException { - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() @@ -174,8 +175,6 @@ void specifyWithNullDatabasesMatchesAll() throws SQLException { @Test void specifyRendersTemplateVariables() throws SQLException { - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() @@ -199,8 +198,6 @@ void specifyRendersTemplateVariables() throws SQLException { @Test void specifyLambdasReturnNonEmptyValues() throws SQLException { // Verify each key field is non-empty. - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() @@ -233,10 +230,6 @@ void specifyLambdasReturnNonEmptyValues() throws SQLException { @Test void specifyWithFlinkConfigPropertiesIncludesThem() throws SQLException { // Verify that sink options ARE merged into the environment - Properties connProps = new Properties(); - connProps.setProperty("flinkConfig1", "value1"); - when(connection.connectionProperties()).thenReturn(connProps); - Map sinkOptions = new HashMap<>(); sinkOptions.put("sinkOption", "sinkVal"); Sink sink = new Sink("sinkdb", Arrays.asList("schema", "sink_table"), sinkOptions); @@ -260,8 +253,6 @@ void specifyWithFlinkConfigPropertiesIncludesThem() throws SQLException { @Test void specifyConditionalRenderedTemplateNotNull() throws SQLException { // Verify null templates are skipped - when(connection.connectionProperties()).thenReturn(new Properties()); - templates.add(new V1alpha1JobTemplate() .metadata(new V1ObjectMeta().name("template1")) .spec(new V1alpha1JobTemplateSpec() diff --git a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java index 3649b0ba4..074a88803 100644 --- a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java +++ b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java @@ -4,18 +4,32 @@ import java.sql.DriverManager; import java.sql.SQLException; import java.sql.SQLNonTransientException; -import java.sql.SQLTransientConnectionException; import java.util.Properties; +import com.linkedin.hoptimator.k8s.K8sContext; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +@ExtendWith(MockitoExtension.class) public class LogicalTableDriverTest { + // K8sContext.create() reads the developer's real ~/.kube/config when no k8s connection + // properties are supplied, which blocks on an interactive cluster login. The failure-path + // tests below stub it via this static mock so they stay hermetic. Validation tests that + // return before reaching K8sContext.create() simply leave it unstubbed. + @Mock + private MockedStatic k8sContextStatic; + @Test public void connectReturnsNullForNonLogicalUrl() throws Exception { LogicalTableDriver driver = new LogicalTableDriver(); @@ -92,39 +106,35 @@ public void connectThrowsWhenDatabasePropertyInUrl() throws Exception { @Test public void connectThrowsNonTransientWhenK8sContextCreationFails() throws Exception { - // All validation passes (2 tiers + database property set) but K8sContext.create() fails - // because kubeconfig points to a non-existent file → catch(Exception) → SQLNonTransientException + // All validation passes (2 tiers + database property set) but K8sContext.create() fails. + // Stub it to throw rather than letting it read the real ~/.kube/config and block on login. + k8sContextStatic.when(() -> K8sContext.create(any())) + .thenThrow(new RuntimeException("simulated K8sContext failure")); + String url = "jdbc:logical://nearline=kafka-database;online=venice"; Properties props = new Properties(); props.setProperty("database", "mylogicaldb"); - props.setProperty("k8s.kubeconfig", "/nonexistent/path/kubeconfig"); - try (Connection ignored = DriverManager.getConnection(url, props)) { - throw new AssertionError("Expected exception"); - } catch (SQLNonTransientException e) { - assertTrue(e.getMessage().contains("Problem loading")); - } + + SQLException ex = assertThrows(SQLNonTransientException.class, + () -> DriverManager.getConnection(url, props)); + assertTrue(ex.getMessage().contains("Problem loading")); } + @Test - void connectWithValidUrlThrowsSQLNonTransientWhenK8sContextFails() { - // A URL that passes all validation (2 tiers + database property) but then fails - // when creating K8sContext (no K8s config in test env) → covered by catch(Exception e) - // at the end of the try block, covering lines 93-103. + void connectPreservesCauseWhenK8sContextCreationFails() { + // The catch(Exception e) branch must wrap the underlying failure as the cause rather than + // swallowing it. Stub K8sContext.create() to throw a known exception and assert it is preserved. + RuntimeException boom = new RuntimeException("boom"); + k8sContextStatic.when(() -> K8sContext.create(any())).thenThrow(boom); + Properties props = new Properties(); props.setProperty("database", "logical"); - try (Connection conn = DriverManager.getConnection( - "jdbc:logical://nearline=kafka-database;online=venice", props)) { - // If connect() unexpectedly succeeds (real K8s available), just verify non-null - assertNotNull(conn); - } catch (SQLNonTransientException e) { - // Expected: K8sContext.create() failed → catch(Exception e) path covered - assertTrue(e.getMessage().contains("Problem loading")); - } catch (SQLTransientConnectionException e) { - // Also acceptable: IOException from K8s client - assertTrue(e.getMessage().contains("Problem loading")); - } catch (SQLException e) { - // Any other SQL exception is also fine — something in connect() path was exercised - assertNotNull(e.getMessage()); - } + + SQLException ex = assertThrows(SQLNonTransientException.class, + () -> DriverManager.getConnection( + "jdbc:logical://nearline=kafka-database;online=venice", props)); + assertTrue(ex.getMessage().contains("Problem loading")); + assertSame(boom, ex.getCause(), "original failure should be preserved as the cause"); } } From 930f5d6f4f1a88dfb02e147a9840d356284eee9d Mon Sep 17 00:00:00 2001 From: Joseph Grogan Date: Mon, 15 Jun 2026 13:22:04 -0400 Subject: [PATCH 2/3] Clean up comments --- .../com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java | 2 -- .../hoptimator/logical/LogicalTableDriverTest.java | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java b/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java index 54da16c8d..c288aaba9 100644 --- a/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java +++ b/hoptimator-k8s/src/test/java/com/linkedin/hoptimator/k8s/K8sJobDeployerTest.java @@ -39,8 +39,6 @@ class K8sJobDeployerTest { // K8sJobDeployer.specify() calls ConfigService.config(context.connection(), ...), which runs the // real ServiceLoader and invokes K8sConfigProvider.loadConfig() -> K8sContext.create(connection). - // Without this static mock, create() would read the developer's real ~/.kube/config and block on - // an interactive cluster login, stalling the unit test. Stubbing it keeps the test hermetic. @Mock private MockedStatic contextStatic; diff --git a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java index 074a88803..2d845222d 100644 --- a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java +++ b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java @@ -23,10 +23,8 @@ @ExtendWith(MockitoExtension.class) public class LogicalTableDriverTest { - // K8sContext.create() reads the developer's real ~/.kube/config when no k8s connection - // properties are supplied, which blocks on an interactive cluster login. The failure-path - // tests below stub it via this static mock so they stay hermetic. Validation tests that - // return before reaching K8sContext.create() simply leave it unstubbed. + // K8sContext.create() reads the real ~/.kube/config (if it exists) when no k8s connection + // properties are supplied. This can cause failures in local testing. Mock it to prevent that. @Mock private MockedStatic k8sContextStatic; @@ -107,7 +105,6 @@ public void connectThrowsWhenDatabasePropertyInUrl() throws Exception { @Test public void connectThrowsNonTransientWhenK8sContextCreationFails() throws Exception { // All validation passes (2 tiers + database property set) but K8sContext.create() fails. - // Stub it to throw rather than letting it read the real ~/.kube/config and block on login. k8sContextStatic.when(() -> K8sContext.create(any())) .thenThrow(new RuntimeException("simulated K8sContext failure")); From cab78d9aafe594204bfc08026df421535561a204 Mon Sep 17 00:00:00 2001 From: Joseph Grogan Date: Mon, 15 Jun 2026 13:26:42 -0400 Subject: [PATCH 3/3] Fix spotbugs --- .../logical/LogicalTableDriverTest.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java index 2d845222d..c62dbc215 100644 --- a/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java +++ b/hoptimator-logical/src/test/java/com/linkedin/hoptimator/logical/LogicalTableDriverTest.java @@ -2,7 +2,6 @@ import java.sql.Connection; import java.sql.DriverManager; -import java.sql.SQLException; import java.sql.SQLNonTransientException; import java.util.Properties; @@ -15,7 +14,6 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -112,13 +110,15 @@ public void connectThrowsNonTransientWhenK8sContextCreationFails() throws Except Properties props = new Properties(); props.setProperty("database", "mylogicaldb"); - SQLException ex = assertThrows(SQLNonTransientException.class, - () -> DriverManager.getConnection(url, props)); - assertTrue(ex.getMessage().contains("Problem loading")); + try (Connection ignored = DriverManager.getConnection(url, props)) { + throw new AssertionError("Expected SQLNonTransientException"); + } catch (SQLNonTransientException e) { + assertTrue(e.getMessage().contains("Problem loading")); + } } @Test - void connectPreservesCauseWhenK8sContextCreationFails() { + void connectPreservesCauseWhenK8sContextCreationFails() throws Exception { // The catch(Exception e) branch must wrap the underlying failure as the cause rather than // swallowing it. Stub K8sContext.create() to throw a known exception and assert it is preserved. RuntimeException boom = new RuntimeException("boom"); @@ -127,11 +127,13 @@ void connectPreservesCauseWhenK8sContextCreationFails() { Properties props = new Properties(); props.setProperty("database", "logical"); - SQLException ex = assertThrows(SQLNonTransientException.class, - () -> DriverManager.getConnection( - "jdbc:logical://nearline=kafka-database;online=venice", props)); - assertTrue(ex.getMessage().contains("Problem loading")); - assertSame(boom, ex.getCause(), "original failure should be preserved as the cause"); + try (Connection ignored = DriverManager.getConnection( + "jdbc:logical://nearline=kafka-database;online=venice", props)) { + throw new AssertionError("Expected SQLNonTransientException"); + } catch (SQLNonTransientException e) { + assertTrue(e.getMessage().contains("Problem loading")); + assertSame(boom, e.getCause(), "original failure should be preserved as the cause"); + } } }