Skip to content

Commit 581633a

Browse files
committed
clean up tests
1 parent 75679cb commit 581633a

File tree

4 files changed

+28
-35
lines changed

4 files changed

+28
-35
lines changed

kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public class CatalogManagedSnapshotManager implements DeltaSnapshotManager, Auto
4242
private final ManagedCatalogAdapter catalogAdapter;
4343
private final String tableId;
4444
private final String tablePath;
45+
private final Configuration hadoopConf;
4546

4647
public CatalogManagedSnapshotManager(
4748
ManagedCatalogAdapter catalogAdapter,
@@ -51,7 +52,7 @@ public CatalogManagedSnapshotManager(
5152
this.catalogAdapter = requireNonNull(catalogAdapter, "catalogAdapter is null");
5253
this.tableId = requireNonNull(tableId, "tableId is null");
5354
this.tablePath = requireNonNull(tablePath, "tablePath is null");
54-
requireNonNull(hadoopConf, "hadoopConf is null");
55+
this.hadoopConf = requireNonNull(hadoopConf, "hadoopConf is null");
5556

5657
logger.info(
5758
"Created CatalogManagedSnapshotManager for table {} at path {}", tableId, tablePath);

kernel-spark/src/main/java/io/delta/kernel/spark/snapshot/unitycatalog/UnityCatalogAdapter.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,14 @@ public final class UnityCatalogAdapter implements ManagedCatalogAdapter {
3737

3838
private final String tableId;
3939
private final String tablePath;
40+
private final String endpoint;
41+
private final String token;
4042

41-
public UnityCatalogAdapter(String tableId, String tablePath) {
43+
public UnityCatalogAdapter(String tableId, String tablePath, String endpoint, String token) {
4244
this.tableId = requireNonNull(tableId, "tableId is null");
4345
this.tablePath = requireNonNull(tablePath, "tablePath is null");
46+
this.endpoint = requireNonNull(endpoint, "endpoint is null");
47+
this.token = requireNonNull(token, "token is null");
4448
}
4549

4650
/**
@@ -59,7 +63,8 @@ public static Optional<ManagedCatalogAdapter> fromCatalog(
5963
/** Creates adapter from connection info (no Spark dependency). */
6064
public static ManagedCatalogAdapter fromConnectionInfo(UnityCatalogConnectionInfo info) {
6165
requireNonNull(info, "info is null");
62-
return new UnityCatalogAdapter(info.getTableId(), info.getTablePath());
66+
return new UnityCatalogAdapter(
67+
info.getTableId(), info.getTablePath(), info.getEndpoint(), info.getToken());
6368
}
6469

6570
public String getTableId() {
@@ -70,6 +75,14 @@ public String getTablePath() {
7075
return tablePath;
7176
}
7277

78+
public String getEndpoint() {
79+
return endpoint;
80+
}
81+
82+
public String getToken() {
83+
return token;
84+
}
85+
7386
@Override
7487
public Snapshot loadSnapshot(
7588
Engine engine, Optional<Long> versionOpt, Optional<Long> timestampOpt) {

kernel-spark/src/test/java/io/delta/kernel/spark/snapshot/DeltaSnapshotManagerFactoryTest.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,30 +46,4 @@ void testFromCatalogTable_NullCatalogTable_ThrowsException() {
4646
() -> DeltaSnapshotManagerFactory.fromCatalogTable(null, null, new Configuration()),
4747
"Null catalogTable should throw NullPointerException");
4848
}
49-
50-
@Test
51-
void testFromCatalogTable_NullHadoopConf_ThrowsException() {
52-
// Can't test without a real CatalogTable instance, so this test validates the pattern
53-
// See integration tests for full validation
54-
}
55-
56-
// Note: Factory behavior tests (which manager type is created) require integration test setup.
57-
// The following tests cannot be implemented as unit tests because the factory requires
58-
// a non-null SparkSession parameter:
59-
//
60-
// - testCreate_NonCatalogManagedTable_ReturnsPathBasedManager: Verify non-UC tables use PathBased
61-
// - testCreate_EmptyCatalogTable_ReturnsPathBasedManager: Verify empty catalogTable uses
62-
// PathBased
63-
// - testCreate_UCManagedTable_ReturnsCatalogManagedManager: Verify UC tables use CatalogManaged
64-
//
65-
// While PathBasedSnapshotManager doesn't technically need SparkSession, the factory API requires
66-
// it to maintain a clean, consistent interface (always available in production via SparkTable).
67-
// Cannot mock SparkSession effectively for these tests.
68-
//
69-
// Note: Testing CatalogManagedSnapshotManager creation requires integration tests with
70-
// real SparkSession and Unity Catalog configuration. This is because:
71-
// 1. CatalogManagedSnapshotManager constructor validates UC table and extracts metadata
72-
// 2. It requires configured UC catalog (spark.sql.catalog.*.uri/token)
73-
// 3. Unit tests cannot easily mock SparkSession's catalog manager
74-
// See CatalogManagedSnapshotManagerTest for integration tests.
7549
}

kernel-spark/src/test/scala/io/delta/kernel/spark/snapshot/CatalogManagedSnapshotManagerSuite.scala

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import io.delta.kernel.CommitRange
2121
import io.delta.kernel.Snapshot
2222
import io.delta.kernel.engine.Engine
2323
import io.delta.kernel.internal.files.ParsedLogData
24-
import io.delta.kernel.spark.snapshot.unitycatalog.UnityCatalogAdapter
24+
2525
import org.apache.hadoop.conf.Configuration
2626
import org.scalatest.funsuite.AnyFunSuite
2727

@@ -46,7 +46,8 @@ class CatalogManagedSnapshotManagerSuite extends AnyFunSuite {
4646
endTimestampOpt: Optional[java.lang.Long]): CommitRange =
4747
throw new UnsupportedOperationException("stub")
4848

49-
override def getRatifiedCommits(endVersionOpt: Optional[java.lang.Long]): java.util.List[ParsedLogData] =
49+
override def getRatifiedCommits(endVersionOpt: Optional[java.lang.Long])
50+
: java.util.List[ParsedLogData] =
5051
throw new UnsupportedOperationException("stub")
5152

5253
override def getLatestRatifiedVersion: Long =
@@ -71,7 +72,8 @@ class CatalogManagedSnapshotManagerSuite extends AnyFunSuite {
7172
}
7273

7374
test("operations are unsupported in wireframe") {
74-
val manager = new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
75+
val manager =
76+
new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
7577

7678
assertThrows[UnsupportedOperationException] {
7779
manager.loadLatestSnapshot()
@@ -91,21 +93,24 @@ class CatalogManagedSnapshotManagerSuite extends AnyFunSuite {
9193
}
9294

9395
test("loadSnapshotAt validates version") {
94-
val manager = new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
96+
val manager =
97+
new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
9598
assertThrows[IllegalArgumentException] {
9699
manager.loadSnapshotAt(-1L)
97100
}
98101
}
99102

100103
test("checkVersionExists validates version") {
101-
val manager = new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
104+
val manager =
105+
new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
102106
assertThrows[IllegalArgumentException] {
103107
manager.checkVersionExists(-1L, true, false)
104108
}
105109
}
106110

107111
test("close is tolerant") {
108-
val manager = new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
112+
val manager =
113+
new CatalogManagedSnapshotManager(stubAdapter, tableId, tablePath, new Configuration())
109114
manager.close() // should not throw
110115
}
111116
}

0 commit comments

Comments
 (0)