hudi-agent commented on code in PR #20361:
URL: https://github.com/apache/hudi/pull/20361#discussion_r4290877670
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestColumnStatsIndexWithSQL.scala:
##########
@@ -42,17 +43,133 @@ import org.apache.spark.sql.catalyst.expressions.{And,
AttributeReference, Bitwi
import org.apache.spark.sql.catalyst.expressions.Literal.TrueLiteral
import org.apache.spark.sql.hudi.DataSkippingUtils
import org.apache.spark.sql.types.StringType
-import org.junit.jupiter.api.{Tag, Test}
+import org.junit.jupiter.api.{AfterEach, BeforeEach, Tag, Test}
import org.junit.jupiter.api.Assertions.{assertEquals, assertFalse, assertTrue}
+import org.junit.jupiter.api.extension.{AfterAllCallback, ExtendWith,
ExtensionContext}
+import org.junit.jupiter.api.parallel.{Execution, ExecutionMode}
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import java.io.File
+import java.lang.ref.WeakReference
+import java.nio.file.{Files, Path, Paths, StandardCopyOption}
import scala.collection.JavaConverters._
+import scala.collection.mutable
@Tag("functional-b")
+@ExtendWith(Array(classOf[TestColumnStatsIndexWithSQL.SharedTablesCleanup]))
+// the shared table cache in the companion object is not thread-safe
+@Execution(ExecutionMode.SAME_THREAD)
class TestColumnStatsIndexWithSQL extends ColumnStatIndexTestBase {
+ import TestColumnStatsIndexWithSQL._
+
+ // Shared tables this test reads or copies; each one is checked for changes
after the test.
+ private var sharedTablesInUse: Seq[SharedTable] = Seq()
+ private var initialSqlConf: Map[String, String] = Map()
+
+ @BeforeEach
+ override def setUp(): Unit = {
+ super.setUp()
+ sharedTablesInUse = Seq()
+ // Every test must start from a fresh SparkSession: no SQL conf, temp
views or cached plans
+ // left over from a previous test that read the same shared table.
+ assertTrue(lastSparkSession.get() ne spark, "SparkSession reused across
tests")
+ lastSparkSession = new WeakReference(spark)
+ assertFalse(spark.catalog.tableExists("tbl"))
+ initialSqlConf = spark.conf.getAll
+ }
+
+ @AfterEach
+ override def tearDown(): Unit = {
+ try {
+ if (spark != null) {
+ spark.catalog.dropTempView("tbl")
+ spark.catalog.clearCache()
+ val sqlConf = spark.sessionState.conf
+ val currentConf = sqlConf.getAllConfs
+ (currentConf.keySet --
initialSqlConf.keySet).foreach(sqlConf.unsetConf)
+ initialSqlConf.foreach { case (k, v) => if
(!currentConf.get(k).contains(v)) sqlConf.setConfString(k, v) }
+ }
+ // A test must never change a shared table (data files, timeline or
metadata table timeline);
+ // tests that write work on their own copy.
+ sharedTablesInUse.foreach { table =>
+ val state = captureTableState(table.path)
+ if (state != table.state) {
+ // evict it so that later tests rebuild a clean table and only this
test fails
+ sharedTables.retain((_, cached) => cached ne table)
+ }
+ assertEquals(table.state, state, s"Shared table ${table.path} was
modified by the test")
+ }
+ } finally {
+ super.tearDown()
+ }
+ }
+
+ /**
+ * Points the test at a table built by `build`, building it only the first
time a test asks for it.
+ * Tables are keyed by `recipe` plus the exact write options, so parameter
cases that write the
+ * same data with the same options (e.g. ones that only differ in how the
index is read) share one table.
+ * `recipe` must name every other input of `build` that affects the table or
its validation.
+ * With `copy = true` the test gets a private copy it may write to.
+ */
Review Comment:
🤖 nit: could you replace the free-form `recipe` strings (e.g.
"setupTable(shouldValidate = false)" repeated across tests) with shared
constants or derive the key from the setupTable arguments? A typo or a drifting
copy would silently give two tests different tables, or worse, the wrong shared
one.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
