[Kernel][UC] Add comprehensive tests for UCManagedCommitClientAdapter with test doubles

Author: TimothyW553

kernel-spark/src/test/scala/io/delta/kernel/spark/catalog/UCManagedCommitClientAdapterSuite.scala

@@ -16,38 +16,223 @@
 
 package io.delta.kernel.spark.catalog
 
+import io.delta.kernel.Snapshot
+import io.delta.kernel.engine.Engine
+import io.delta.kernel.unitycatalog.UCCatalogManagedClient
+import io.delta.storage.commit.{Commit, GetCommitsResponse}
+import io.delta.storage.commit.uccommitcoordinator.UCClient
+import java.io.IOException
+import java.net.URI
+import java.util.Optional
 import org.scalatest.funsuite.AnyFunSuite
 
 /**
- * Basic validation tests for [[UCManagedCommitClientAdapter]].
+ * Tests for [[UCManagedCommitClientAdapter]].
  *
- * Note: This adapter is a simple delegation wrapper with no business logic.
- * Comprehensive testing happens through integration tests with real UC tables.
- * These tests just verify basic construction requirements.
+ * Uses simple test doubles instead of mocking frameworks to validate
+ * adapter behavior including error handling and edge cases.
  */
 class UCManagedCommitClientAdapterSuite extends AnyFunSuite {
 
-  test("constructor validates non-null parameters") {
-    // The constructor uses requireNonNull which throws NullPointerException
-    // for null arguments. We verify this requirement is enforced.
+  // Test double for UCClient
+  class TestUCClient extends UCClient {
+    var closeCalled = false
+    var getCommitsCalled = 0
+    var latestVersionToReturn: Long = 5
+    var shouldThrowOnGetCommits = false
+    var commitsToReturn: java.util.List[Commit] = java.util.Collections.emptyList()
 
-    // This test documents the contract without needing to instantiate
-    // actual UC clients (which would require test infrastructure)
-    assert(classOf[UCManagedCommitClientAdapter] != null)
+    override def getMetastoreId(): String = "test-metastore"
 
-    // The adapter implements ManagedCommitClient interface
-    val interfaces = classOf[UCManagedCommitClientAdapter].getInterfaces
-    assert(interfaces.exists(_.getName == "io.delta.kernel.spark.catalog.ManagedCommitClient"))
+    override def commit(
+        tableId: String,
+        tableUri: URI,
+        commit: Optional[Commit],
+        lastKnownBackfilledVersion: Optional[java.lang.Long],
+        disown: Boolean,
+        newMetadata: Optional[io.delta.storage.commit.actions.AbstractMetadata],
+        newProtocol: Optional[io.delta.storage.commit.actions.AbstractProtocol]): Unit = {
+      throw new UnsupportedOperationException("Not used in adapter tests")
+    }
+
+    override def getCommits(
+        tableId: String,
+        tableUri: URI,
+        startVersion: Optional[java.lang.Long],
+        endVersion: Optional[java.lang.Long]): GetCommitsResponse = {
+      getCommitsCalled += 1
+      if (shouldThrowOnGetCommits) {
+        throw new IOException("Test exception")
+      }
+      new GetCommitsResponse(commitsToReturn, latestVersionToReturn)
+    }
+
+    override def close(): Unit = {
+      closeCalled = true
+    }
+  }
+
+  // Test double for UCCatalogManagedClient
+  class TestUCCatalogManagedClient(client: UCClient) extends UCCatalogManagedClient(client) {
+    var loadSnapshotCalled = 0
+    var lastEnginePassedIn: Engine = _
+    var lastTableIdPassedIn: String = _
+    var lastTablePathPassedIn: String = _
+    var lastVersionPassedIn: Optional[java.lang.Long] = _
+    var lastTimestampPassedIn: Optional[java.lang.Long] = _
+    var snapshotToReturn: Snapshot = _
+
+    override def loadSnapshot(
+        engine: Engine,
+        ucTableId: String,
+        tablePath: String,
+        versionOpt: Optional[java.lang.Long],
+        timestampOpt: Optional[java.lang.Long]): Snapshot = {
+      loadSnapshotCalled += 1
+      lastEnginePassedIn = engine
+      lastTableIdPassedIn = ucTableId
+      lastTablePathPassedIn = tablePath
+      lastVersionPassedIn = versionOpt
+      lastTimestampPassedIn = timestampOpt
+      snapshotToReturn
+    }
+  }
+
+
+  test("constructor validates non-null ucClient") {
+    val testUCClient = new TestUCClient()
+    assertThrows[NullPointerException] {
+      new UCManagedCommitClientAdapter(null, testUCClient, "/tmp/test")
+    }
+  }
+
+  test("constructor validates non-null rawUCClient") {
+    val testUCClient = new TestUCClient()
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    assertThrows[NullPointerException] {
+      new UCManagedCommitClientAdapter(testUCCatalogClient, null, "/tmp/test")
+    }
+  }
+
+  test("constructor validates non-null tablePath") {
+    val testUCClient = new TestUCClient()
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    assertThrows[NullPointerException] {
+      new UCManagedCommitClientAdapter(testUCCatalogClient, testUCClient, null)
+    }
+  }
+
+  test("getSnapshot delegates to UCCatalogManagedClient with correct parameters") {
+    val testUCClient = new TestUCClient()
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    // Use null as snapshot - we're only testing delegation, not snapshot behavior
+    testUCCatalogClient.snapshotToReturn = null
+
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    val mockEngine = null.asInstanceOf[Engine]  // Not used by test double
+    adapter.getSnapshot(
+      mockEngine,
+      "test-table-id",
+      "/tmp/test/table",
+      Optional.empty(),
+      Optional.empty())
+
+    // Verify delegation happened with correct parameters
+    assert(testUCCatalogClient.loadSnapshotCalled == 1)
+    assert(testUCCatalogClient.lastTableIdPassedIn == "test-table-id")
+    assert(testUCCatalogClient.lastTablePathPassedIn == "/tmp/test/table")
+    assert(!testUCCatalogClient.lastVersionPassedIn.isPresent)
+    assert(!testUCCatalogClient.lastTimestampPassedIn.isPresent)
+  }
+
+  test("versionExists returns true when version exists") {
+    val testUCClient = new TestUCClient()
+    testUCClient.latestVersionToReturn = 10
+    testUCClient.commitsToReturn = java.util.Collections.emptyList()
+
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    assert(adapter.versionExists("test-table-id", 5))
+    assert(testUCClient.getCommitsCalled == 1)
+  }
+
+  test("versionExists returns false when UCClient throws exception") {
+    val testUCClient = new TestUCClient()
+    testUCClient.shouldThrowOnGetCommits = true
+
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    assert(!adapter.versionExists("test-table-id", 5))
+    assert(testUCClient.getCommitsCalled == 1)
+  }
+
+  test("getLatestVersion returns latestTableVersion from UC") {
+    val testUCClient = new TestUCClient()
+    testUCClient.latestVersionToReturn = 42
+
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    val result = adapter.getLatestVersion("test-table-id")
+    assert(result == 42)
+    assert(testUCClient.getCommitsCalled == 1)
+  }
+
+  test("getLatestVersion converts -1 to 0 for newly created tables") {
+    val testUCClient = new TestUCClient()
+    testUCClient.latestVersionToReturn = -1  // UC returns -1 when only 0.json exists
+
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    val result = adapter.getLatestVersion("test-table-id")
+    assert(result == 0)  // Should convert -1 to 0
+  }
+
+  test("getLatestVersion throws RuntimeException when UCClient fails") {
+    val testUCClient = new TestUCClient()
+    testUCClient.shouldThrowOnGetCommits = true
+
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
+
+    val exception = intercept[RuntimeException] {
+      adapter.getLatestVersion("test-table-id")
+    }
+    assert(exception.getMessage.contains("Failed to get latest version"))
   }
 
-  test("adapter class has expected methods") {
-    val methods = classOf[UCManagedCommitClientAdapter].getDeclaredMethods
-    val methodNames = methods.map(_.getName).toSet
+  test("close delegates to UCClient") {
+    val testUCClient = new TestUCClient()
+    val testUCCatalogClient = new TestUCCatalogManagedClient(testUCClient)
+    val adapter = new UCManagedCommitClientAdapter(
+      testUCCatalogClient,
+      testUCClient,
+      "/tmp/test/table")
 
-    // Verify the adapter implements required interface methods
-    assert(methodNames.contains("getSnapshot"))
-    assert(methodNames.contains("versionExists"))
-    assert(methodNames.contains("getLatestVersion"))
-    assert(methodNames.contains("close"))
+    assert(!testUCClient.closeCalled)
+    adapter.close()
+    assert(testUCClient.closeCalled)
   }
 }