Skip to content

Commit b751374

Browse files
zifeif2anishshri-db
authored andcommitted
[SPARK-55151] Fix RocksDBSuite testWithStateStoreCheckpointIds
### What changes were proposed in this pull request? In ScalaTest, test expects a parameterless lambda (() => Any). However, the helper testWithStateStoreCheckpointIds incorrectly passed a lambda with a Boolean parameter into test. As a result, the test body was never executed and was instead treated as a literal function value, causing tests to silently not run. This PR fixes the helper function by ensuring the lambda passed to test has no parameters, while still correctly threading the enableStateStoreCheckpointIds flag into the test logic. It also updates affected tests that began failing after the fix to reflect the corrected behavior. Tests including - loadEmpty tests ### Why are the changes needed? So that future engineers who uses this util function will actually get their tests ran ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? This [run](https://github.com/zifeif2/spark/actions/runs/21275903248/job/61235478518?pr=7) was failing if we only fix the helper function, but the fixed unit tests are passing in this PR too ### Was this patch authored or co-authored using generative AI tooling? No Closes #53936 from zifeif2/zifeif2-fix-rocksdb-suite. Lead-authored-by: Zifei Feng <zifeifeng11@gmail.com> Co-authored-by: zifeif2 <zifeifeng11@gmail.com> Signed-off-by: Anish Shrigondekar <anish.shrigondekar@databricks.com>
1 parent dec114a commit b751374

File tree

1 file changed

+14
-4
lines changed

1 file changed

+14
-4
lines changed

sql/core/src/test/scala/org/apache/spark/sql/execution/streaming/state/RocksDBSuite.scala

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ trait AlsoTestWithRocksDBFeatures
276276
Seq(true, false).foreach { enableStateStoreCheckpointIds =>
277277
val newTestName = s"$testName - with enableStateStoreCheckpointIds = " +
278278
s"$enableStateStoreCheckpointIds"
279-
test(newTestName, testTags: _*) { enableStateStoreCheckpointIds =>
279+
test(newTestName, testTags: _*) {
280280
testBody(enableStateStoreCheckpointIds)
281281
}
282282
}
@@ -3286,7 +3286,10 @@ class RocksDBSuite extends AlsoTestWithRocksDBFeatures with SharedSparkSession
32863286
// upload snapshot 4.zip
32873287
db.doMaintenance()
32883288
}
3289-
withDB(remoteDir, version = 4, conf = conf) { db =>
3289+
withDB(remoteDir, version = 4, conf = conf,
3290+
enableStateStoreCheckpointIds = enableStateStoreCheckpointIds,
3291+
versionToUniqueId = versionToUniqueId) { db =>
3292+
db.close()
32903293
}
32913294
})
32923295
}
@@ -3315,7 +3318,10 @@ class RocksDBSuite extends AlsoTestWithRocksDBFeatures with SharedSparkSession
33153318
db.doMaintenance()
33163319
}
33173320

3318-
withDB(remoteDir, version = 4, conf = conf) { db =>
3321+
withDB(remoteDir, version = 4, conf = conf,
3322+
enableStateStoreCheckpointIds = enableStateStoreCheckpointIds,
3323+
versionToUniqueId = versionToUniqueId) { db =>
3324+
db.close()
33193325
}
33203326
}
33213327

@@ -4177,7 +4183,11 @@ class RocksDBSuite extends AlsoTestWithRocksDBFeatures with SharedSparkSession
41774183
// So still do a versionToUniqueId.get
41784184
ckptId match {
41794185
case Some(_) => super.load(version, ckptId, readOnly, loadEmpty)
4180-
case None => super.load(version, versionToUniqueId.get(version), readOnly, loadEmpty)
4186+
case None => super.load(
4187+
version,
4188+
if (!loadEmpty) versionToUniqueId.get(version) else None,
4189+
readOnly,
4190+
loadEmpty)
41814191
}
41824192
}
41834193

0 commit comments

Comments
 (0)