Skip to content

Commit 188bb4b

Browse files
jogroganclaude
andauthored
Backfill test coverage to 80%+ per-file and improve test quality (#203)
* Backfill tests to reach 80% coverage goal * imports * cleanup various inspection problems * Some cleanup, consolidation, and missing files * Kill surviving PiTest mutations in kafka, venice, and mysql modules Added 78 targeted tests across 13 test files to kill mutations that survived the previous backfill: - hoptimator-kafka (+15 tests): createTopic retention config branching, restore cleanup, update-partitions boundary, validate boundary cases, provider type-guard, driver autocommit/schema registration, and ClusterSchema description content - hoptimator-venice (+19 tests): validate() null/empty/error branches, createControllerClient SSL vs non-SSL paths, ClusterSchema loadAllTables content and getSslFactory, Provider instanceof guard, Driver autocommit and catalog assignment - hoptimator-mysql (+44 tests): exact MySQL DDL type strings per SQL type, VARCHAR precision boundary, buildCreateTableSql nullable/PK/length DDL, alterTable ADD/MODIFY/DROP COLUMN, escapeIdentifier backtick output, ensureDatabaseExists DDL, validate schema mismatch detection, Provider instanceof guard, Driver autocommit, MySqlTable nullability branching, TableSchema description content All 286 tests pass (kafka: 64, venice: 88, mysql: 134). Coverage: 85.95% line / 82.07% branch overall. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Remove 17 redundant tests made obsolete by mutation-killing backfill Semantic and structural audit across all test files identified tests that are strict subsets of newer, more specific mutation-killing tests: - SourceTest: testSchemaReturnsNullForSingleElementPath (→ ExactlyOneElement version) - DeployerUtilsTest: testParseIntOptionReturnsDefault/CustomValue, testParseLongOptionReturnsCustomValue (→ KeyAbsent/KeyPresent variants) - HoptimatorDatabaseMetaDataTest: testGetSchemasReturnsResultSet (→ NoArgs), testGetSchemasWithSchemaPattern (→ WithNonNullSchemaPatternUsingRegex) - KafkaDeployerTest: testCreateNewTopicWithRetention (→ PassesRetentionToNewTopic, which also asserts exact RETENTION_MS_CONFIG value via ArgumentCaptor) - KafkaDriverTest: connectAutoCommitIsSetToTrue, connectAddsKafkaSchemaToRootSchema (assertions already present in connectWithCorrectPrefixReturnsConnection/ connectRegistersKafkaSchema) - VeniceDeployerValidationTest: testSpecifyReturnsEmptyList (exact duplicate in VeniceDeployerTest) - VeniceDriverTest: testConnectSetsAutoCommitTrue, testConnectSetsCatalogToVenice (already asserted in connectWithCorrectPrefixReturnsConnection) - MySqlDeployerTest: testValidatePassesForNewTable (→ testValidatePassesWhenAllConditionsGood with fuller setup) - MySqlDeployerValidationTest: testValidateFailsWhenKeyFieldTypeMismatch (weaker than testValidateFailsWhenKeyFieldTypeChanges which also checks field name and types); testToMySqlTypeMappings (→ testToMySqlTypeExactSqlString with ArgumentCaptor) - MySqlDeployerProviderTest: testDeployersReturnsExactlyOneMySqlDeployer (identical to testReturnsDeployerForMySqlSchema) - MySqlDriverTest: testConnectSetsAutoCommitToTrue (already in correctPrefixReturnsConnection) - MySqlTableTest: testGetRowTypeWithEmptyTableReturnsZeroColumns (exact duplicate of testGetRowTypeWithEmptyTable) Build and all tests still pass. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Refactor test structure: extract, consolidate, and clean up CheckResponseMutationTests → K8sApiErrorResponseTest: The nested class had fully independent @mock fields and @beforeeach setup; extracted to its own top-level file K8sApiErrorResponseTest.java. All 7 error-response tests now live at the proper scope. TableTriggerReconcilerMockTest + TestTableTriggerReconciler → TableTriggerReconcilerTest: Two files testing the same class merged into one. State-based tests (ex-TestTableTriggerReconciler) are top-level; interaction/spy-based tests (ex-TableTriggerReconcilerMockTest) live in a @nested static InteractionTests class. Both old files deleted. FakeK8sApi: removed System.out.println noise from create/delete/update. java.lang.reflect removed: Added PipelineRel.Implementor.hasSink() package accessor; replaced three-line reflection block in HoptimatorDdlUtilsTest with assertTrue(plan.hasSink()). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Fix test compilation and stubbing issues after rebase - AvroConverterTypeMappingTest: update null-type assertion from SqlTypeName.UNKNOWN to SqlTypeName.NULL to match production code added in the main-branch AvroConverter null-guard commit - K8sPipelineElementStatusEstimatorTest: add setUpPipelineMocks() to tests that need pipeline/context stubs after setUp() was refactored to not include them; remove unused lenient import; replace setUpPipelineMocks() with minimal stubs in testEstimateStatusesWithEmptyYamlSpec to avoid UnnecessaryStubbingException Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Normalize test class naming: Test* → *Test where a source class exists Renamed (git tracks as moves): - TestValidatorProvider → ValidatorProviderTest (corresponds to ValidatorProvider interface) - TestControllerProvider → ControllerProviderTest (corresponds to ControllerProvider interface) Updated ServiceLoader registrations and references in ValidationServiceTest, ControllerServiceTest accordingly. Test* prefix kept for TestSqlScripts and TestBasicSql — these are integration- style SQL script runners with no corresponding source class, so Test* is appropriate for them. Merged unique tests from old Test* files into existing *Test counterparts, then deleted the old files: - TestK8sSnapshot (2 tests) → merged into K8sSnapshotTest - TestDataTypeUtils (3 tests) → merged into DataTypeUtilsTest - TestTemplate (4 unique tests) → merged into TemplateTest Deleted as fully superseded: - TestK8sConnector (covered by K8sConnectorTest) - TestViewReconciler (covered by ViewReconcilerTest) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Test owner reference logic added in PR 204 Two new tests in TableTriggerReconcilerTest.InteractionTests cover both branches of the owner reference selection added in Spawn Trigger Jobs With Owner Reference: - jobCreatedWithSyntheticOwnerReferenceWhenTriggerHasNone: trigger has no ownerReferences, createWithMetadata receives a synthetic ref built from the trigger own apiVersion, kind, name, and uid - jobCreatedWithExistingOwnerReferencesWhenTriggerHasThem: trigger has existing ownerReferences, they are passed through unchanged Previously all createWithMetadata verifications used any() for the owner reference argument and did not exercise this logic. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 51f1ba0 commit 188bb4b

317 files changed

Lines changed: 32802 additions & 2053 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/integration-tests.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ on:
88

99
permissions:
1010
contents: read
11+
pull-requests: write
1112

1213
jobs:
1314
build:
@@ -23,6 +24,17 @@ jobs:
2324
distribution: 'temurin'
2425
- name: Build
2526
run: make build
27+
- name: Jacoco Report to PR
28+
id: jacoco
29+
uses: madrapps/jacoco-report@v1.7.2
30+
with:
31+
paths: ${{ github.workspace }}/build/reports/jacoco/aggregate/jacocoAggregateReport.xml
32+
token: ${{ secrets.GITHUB_TOKEN }}
33+
title: Code Coverage
34+
min-coverage-overall: 40
35+
min-coverage-changed-files: 60
36+
pass-emoji: ':green_circle:'
37+
update-comment: true
2638
- name: Create K8s Kind Cluster
2739
uses: helm/kind-action@v1.4.0
2840
with:

Makefile

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ install:
55
test:
66
./gradlew test -x spotbugsMain -x spotbugsTest -x spotbugsTestFixtures
77

8+
coverage:
9+
./gradlew jacocoAggregateReport -x spotbugsMain -x spotbugsTest -x spotbugsTestFixtures
10+
@echo "Aggregate report: file://$(shell pwd)/build/reports/jacoco/aggregate/index.html"
11+
812
build:
9-
./gradlew build shadowJar
13+
./gradlew build jacocoAggregateReport shadowJar
1014
docker build . -t hoptimator
1115
docker build hoptimator-flink-runner -f hoptimator-flink-runner/Dockerfile-flink-runner -t hoptimator-flink-runner
1216
docker build hoptimator-flink-runner -f hoptimator-flink-runner/Dockerfile-flink-operator -t hoptimator-flink-operator
@@ -149,4 +153,4 @@ run-zeppelin: build-zeppelin
149153
--name hoptimator-zeppelin \
150154
hoptimator-zeppelin
151155

152-
.PHONY: install test build bounce clean quickstart deploy-config undeploy-config deploy undeploy deploy-demo undeploy-demo deploy-flink undeploy-flink deploy-kafka undeploy-kafka deploy-mysql undeploy-mysql deploy-venice undeploy-venice build-zeppelin run-zeppelin integration-tests integration-tests-kind deploy-dev-environment undeploy-dev-environment generate-models release
156+
.PHONY: install test coverage build bounce clean quickstart deploy-config undeploy-config deploy undeploy deploy-demo undeploy-demo deploy-flink undeploy-flink deploy-kafka undeploy-kafka deploy-mysql undeploy-mysql deploy-venice undeploy-venice build-zeppelin run-zeppelin integration-tests integration-tests-kind deploy-dev-environment undeploy-dev-environment generate-models release

build.gradle

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,57 @@ plugins {
22
id("com.github.spotbugs") version "6.1.10"
33
}
44

5+
apply plugin: 'jacoco'
6+
7+
// All JaCoCo exclusions for the aggregate report, centralised here.
8+
// Per-module jacocoTestReport exclusions don't reliably propagate to the aggregate
9+
// task because of Gradle's configuration resolution order, so all exclusions are
10+
// declared once and applied when building filtered FileTrees below.
11+
ext.jacocoAggregateExcludes = [
12+
// ── hoptimator-models / hoptimator-k8s ──────────────────────────────────────
13+
'**/models/**', // auto-generated K8s CRD model classes
14+
15+
// ── hoptimator-operator ──────────────────────────────────────────────────────
16+
'**/PipelineOperatorApp.class', // operator main entry point
17+
18+
// ── hoptimator-flink-runner ──────────────────────────────────────────────────
19+
'**/FlinkRunner.class', // CLI entry point; requires Flink runtime
20+
21+
// ── hoptimator-cli ───────────────────────────────────────────────────────────
22+
'sqlline/**', // third-party library bundled via application
23+
24+
// ── hoptimator-jdbc ──────────────────────────────────────────────────────────
25+
'**/ddl/**', // auto-generated JavaCC SQL parser
26+
]
27+
28+
task jacocoAggregateReport(type: JacocoReport) {
29+
group = 'verification'
30+
description = 'Generates an aggregate JaCoCo coverage report across all subprojects.'
31+
reports {
32+
html.required = true
33+
xml.required = true
34+
html.outputLocation = layout.buildDirectory.dir('reports/jacoco/aggregate')
35+
xml.outputLocation = layout.buildDirectory.file('reports/jacoco/aggregate/jacocoAggregateReport.xml')
36+
}
37+
}
38+
39+
subprojects {
40+
plugins.withType(JacocoPlugin).configureEach {
41+
rootProject.jacocoAggregateReport.dependsOn(test)
42+
rootProject.jacocoAggregateReport.executionData(test)
43+
rootProject.jacocoAggregateReport.sourceDirectories.from(sourceSets.main.allSource.srcDirs)
44+
// Add a filtered FileTree for each classes directory. Using fileTree directly (rather than
45+
// jacocoTestReport.classDirectories) ensures the exclusions are applied at resolution time,
46+
// bypassing Gradle's configuration-resolution ordering issue with per-module setFrom().
47+
def excludes = rootProject.ext.jacocoAggregateExcludes
48+
sourceSets.main.output.classesDirs.each { classesDir ->
49+
rootProject.jacocoAggregateReport.classDirectories.from(
50+
fileTree(dir: classesDir, excludes: excludes)
51+
)
52+
}
53+
}
54+
}
55+
556
subprojects {
657
apply plugin: 'checkstyle'
758
apply plugin: 'com.github.spotbugs'
@@ -18,12 +69,18 @@ subprojects {
1869
spotbugs {
1970
ignoreFailures = false
2071
showProgress = true
21-
reportsDir = file("$rootProject.layout.buildDirectory/spotbugs")
72+
reportsDir = file("${rootDir}/build/spotbugs")
2273
includeFilter = file("$rootDir/config/spotbugs/include.xml")
2374
excludeFilter = file("$rootDir/config/spotbugs/exclude.xml")
2475
}
2576

2677
plugins.withType(JavaPlugin) {
78+
project.apply plugin: 'jacoco'
79+
80+
jacoco {
81+
toolVersion = "0.8.11"
82+
}
83+
2784
dependencies {
2885
testImplementation libs.assertj
2986
testImplementation libs.junit.jupiter.api
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package com.linkedin.hoptimator;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
import java.util.Map;
8+
import java.util.Set;
9+
10+
import static org.junit.jupiter.api.Assertions.assertEquals;
11+
import static org.junit.jupiter.api.Assertions.assertThrows;
12+
13+
14+
class JobTest {
15+
16+
@Test
17+
void testNameAndAccessors() {
18+
Sink sink = new Sink("db", List.of("s", "t"), Collections.emptyMap());
19+
Source src = new Source("db", List.of("s", "src"), Collections.emptyMap());
20+
Set<Source> sources = Set.of(src);
21+
Map<String, ThrowingFunction<SqlDialect, String>> evals = Map.of(
22+
"sql", d -> "INSERT INTO t SELECT * FROM src",
23+
"query", d -> "SELECT * FROM src",
24+
"fieldMap", d -> "{}"
25+
);
26+
27+
Job job = new Job("myJob", sources, sink, evals);
28+
29+
assertEquals("myJob", job.name());
30+
assertEquals(sources, job.sources());
31+
assertEquals(sink, job.sink());
32+
}
33+
34+
@Test
35+
void testSqlReturnsFunction() throws Exception {
36+
Sink sink = new Sink("db", List.of("t"), Collections.emptyMap());
37+
Map<String, ThrowingFunction<SqlDialect, String>> evals = Map.of(
38+
"sql", d -> "my-sql",
39+
"query", d -> "my-query",
40+
"fieldMap", d -> "my-fields"
41+
);
42+
Job job = new Job("j", Collections.emptySet(), sink, evals);
43+
44+
assertEquals("my-sql", job.sql().apply(SqlDialect.ANSI));
45+
assertEquals("my-query", job.query().apply(SqlDialect.FLINK));
46+
assertEquals("my-fields", job.fieldMap().apply(SqlDialect.ANSI));
47+
}
48+
49+
@Test
50+
void testEvalThrowsForUnknownKey() {
51+
Sink sink = new Sink("db", List.of("t"), Collections.emptyMap());
52+
Map<String, ThrowingFunction<SqlDialect, String>> evals = Map.of(
53+
"sql", d -> "s"
54+
);
55+
Job job = new Job("j", Collections.emptySet(), sink, evals);
56+
57+
assertThrows(IllegalArgumentException.class, job::query);
58+
}
59+
60+
@Test
61+
void testToString() {
62+
Sink sink = new Sink("db", List.of("schema", "table"), Collections.emptyMap());
63+
Job job = new Job("j", Collections.emptySet(), sink, Collections.emptyMap());
64+
assertEquals("Job[schema.table]", job.toString());
65+
}
66+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package com.linkedin.hoptimator;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertNotNull;
10+
11+
12+
class MaterializedViewTest {
13+
14+
@Test
15+
void testAccessors() {
16+
Sink sink = new Sink("db", List.of("sink"), Collections.emptyMap());
17+
Job job = new Job("j", Collections.emptySet(), sink, Collections.emptyMap());
18+
Pipeline pipeline = new Pipeline(Collections.emptyList(), sink, job);
19+
ThrowingFunction<SqlDialect, String> pSql = d -> "INSERT INTO sink SELECT * FROM src";
20+
21+
MaterializedView mv = new MaterializedView("myDb", List.of("s", "v"), "SELECT 1", pSql, pipeline);
22+
23+
assertEquals("myDb", mv.database());
24+
assertEquals(pipeline, mv.pipeline());
25+
assertNotNull(mv.pipelineSql());
26+
assertEquals("SELECT 1", mv.viewSql());
27+
assertEquals("v", mv.table());
28+
assertEquals("s", mv.schema());
29+
}
30+
31+
@Test
32+
void testToString() {
33+
MaterializedView mv = new MaterializedView("db", List.of("a", "b"), "SELECT 1", d -> "", null);
34+
assertEquals("MaterializedView[a.b]", mv.toString());
35+
}
36+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.linkedin.hoptimator;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertNotNull;
10+
11+
12+
class PipelineTest {
13+
14+
@Test
15+
void testAccessors() {
16+
Source source = new Source("db", List.of("src"), Collections.emptyMap());
17+
Sink sink = new Sink("db", List.of("sink"), Collections.emptyMap());
18+
Job job = new Job("j", Collections.emptySet(), sink, Collections.emptyMap());
19+
List<Source> sources = List.of(source);
20+
21+
Pipeline pipeline = new Pipeline(sources, sink, job);
22+
23+
assertEquals(sources, List.copyOf(pipeline.sources()));
24+
assertEquals(sink, pipeline.sink());
25+
assertEquals(job, pipeline.job());
26+
assertNotNull(pipeline.sources());
27+
}
28+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package com.linkedin.hoptimator;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
10+
11+
class SinkTest {
12+
13+
@Test
14+
void testToString() {
15+
Sink sink = new Sink("db", List.of("schema", "table"), Collections.emptyMap());
16+
assertEquals("Sink[schema.table]", sink.toString());
17+
}
18+
19+
@Test
20+
void testInheritsSourceBehavior() {
21+
Sink sink = new Sink("myDb", List.of("cat", "sch", "tbl"), Collections.emptyMap());
22+
assertEquals("tbl", sink.table());
23+
assertEquals("sch", sink.schema());
24+
assertEquals("cat", sink.catalog());
25+
assertEquals("myDb", sink.database());
26+
}
27+
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package com.linkedin.hoptimator;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import java.util.Collections;
6+
import java.util.List;
7+
import java.util.Map;
8+
9+
import static org.junit.jupiter.api.Assertions.assertEquals;
10+
import static org.junit.jupiter.api.Assertions.assertNull;
11+
12+
13+
class SourceTest {
14+
15+
@Test
16+
void testTableReturnsLastPathElement() {
17+
Source source = new Source("db", List.of("catalog", "schema", "myTable"), Collections.emptyMap());
18+
assertEquals("myTable", source.table());
19+
}
20+
21+
@Test
22+
void testSchemaReturnsPenultimateElement() {
23+
Source source = new Source("db", List.of("catalog", "schema", "myTable"), Collections.emptyMap());
24+
assertEquals("schema", source.schema());
25+
}
26+
27+
@Test
28+
void testCatalogReturnsThirdFromEnd() {
29+
Source source = new Source("db", List.of("catalog", "schema", "myTable"), Collections.emptyMap());
30+
assertEquals("catalog", source.catalog());
31+
}
32+
33+
@Test
34+
void testCatalogReturnsNullForTwoElementPath() {
35+
Source source = new Source("db", List.of("schema", "myTable"), Collections.emptyMap());
36+
assertNull(source.catalog());
37+
}
38+
39+
@Test
40+
void testDatabase() {
41+
Source source = new Source("myDb", List.of("t"), Collections.emptyMap());
42+
assertEquals("myDb", source.database());
43+
}
44+
45+
@Test
46+
void testOptions() {
47+
Map<String, String> opts = Map.of("key", "value");
48+
Source source = new Source("db", List.of("t"), opts);
49+
assertEquals(opts, source.options());
50+
}
51+
52+
@Test
53+
void testPath() {
54+
List<String> path = List.of("a", "b", "c");
55+
Source source = new Source("db", path, Collections.emptyMap());
56+
assertEquals(path, source.path());
57+
}
58+
59+
@Test
60+
void testPathString() {
61+
Source source = new Source("db", List.of("a", "b", "c"), Collections.emptyMap());
62+
assertEquals("a.b.c", source.pathString());
63+
}
64+
65+
@Test
66+
void testToString() {
67+
Source source = new Source("db", List.of("a", "b"), Collections.emptyMap());
68+
assertEquals("Source[a.b]", source.toString());
69+
}
70+
71+
@Test
72+
void testSchemaReturnsValueForExactlyTwoElementPath() {
73+
Source source = new Source("db", List.of("mySchema", "myTable"), Collections.emptyMap());
74+
assertEquals("mySchema", source.schema(),
75+
"schema() must return the second-to-last element when path has exactly 2 elements");
76+
}
77+
78+
@Test
79+
void testSchemaReturnsNullForExactlyOneElementPath() {
80+
Source source = new Source("db", List.of("onlyTable"), Collections.emptyMap());
81+
assertNull(source.schema(),
82+
"schema() must return null when path has only 1 element");
83+
}
84+
}

0 commit comments

Comments
 (0)