andygrove commented on code in PR #7110:
URL: https://github.com/apache/datafusion-comet/pull/7110#discussion_r4543603267
##########
spark/src/test/scala/org/apache/spark/sql/CometTPCDSQueryTestSuite.scala:
##########
@@ -210,7 +210,13 @@ class CometTPCDSQueryTestSuite extends QueryTest with
TPCDSBase with CometSQLQue
allJoinConfCombinations.contains(conf),
s"Join configurations [$conf] should be one of
$allJoinConfCombinations"))
- if (tpcdsDataPath.nonEmpty) {
+ /**
+ * Registers the TPC-DS tests. A subclass that overrides `tpcdsQueries` must
call this at the
+ * end of its own body, once its overrides are initialized. Calling it from
this class's
+ * constructor would see them as null, which previously required an early
initializer in the
+ * subclass.
+ */
+ protected def registerTpcdsQueryTests(): Unit = if (tpcdsDataPath.nonEmpty) {
Review Comment:
Good catch. The shared registration now lives in an abstract
`CometTPCDSQueryTestBase`, and `CometTPCDSQueryTestSuite` and
`CometTPCDSQuerySuite` each call `registerTpcdsQueryTests()` at the end of
their own body. I checked it with 24 empty table directories as
`SPARK_TPCDS_DATA`, as in your harness: the parent suite registers 130 tests,
`q39a` and `q39b` included, and the child registers 134. I also merged `main`,
which moved the Celeborn frame helpers into `NativeShuffleTestFrames`, so the
`Array`-taking `frameArray` that avoids the varargs copy is there now.
`test-compile -Pspark-3.5 -Pstrict-warnings` passes (552f689f7f).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
