andygrove commented on code in PR #2505:
URL: https://github.com/apache/datafusion-comet/pull/2505#discussion_r2393330944
##########
spark/src/test/scala/org/apache/spark/sql/comet/CometPlanStabilitySuite.scala:
##########
@@ -131,38 +133,54 @@ trait CometPlanStabilitySuite extends
DisableAdaptiveExecutionSuite with TPCDSBa
}
}
- private def checkWithApproved(plan: SparkPlan, name: String, explain:
String): Unit = {
+ private def checkWithApproved(plan: SparkPlan, name: String, actualExplain:
String): Unit = {
val dir = getDirForTest(name)
val tempDir = FileUtils.getTempDirectory
val actualSimplified = getSimplifiedPlan(plan)
- val foundMatch = isApproved(dir, actualSimplified, explain)
+ val foundMatch = isApproved(dir, actualSimplified, actualExplain)
if (!foundMatch) {
- // show diff with last approved
+ // read approved files
val approvedSimplifiedFile = new File(dir, "simplified.txt")
val approvedExplainFile = new File(dir, "explain.txt")
+ val approvedSimplified =
+ FileUtils.readFileToString(approvedSimplifiedFile,
StandardCharsets.UTF_8)
+ val approvedExplain =
+ FileUtils.readFileToString(approvedExplainFile, StandardCharsets.UTF_8)
+ // write actual files out for debugging
val actualSimplifiedFile = new File(tempDir,
s"$name.actual.simplified.txt")
val actualExplainFile = new File(tempDir, s"$name.actual.explain.txt")
-
- val approvedSimplified =
- FileUtils.readFileToString(approvedSimplifiedFile,
StandardCharsets.UTF_8)
- // write out for debugging
FileUtils.writeStringToFile(actualSimplifiedFile, actualSimplified,
StandardCharsets.UTF_8)
- FileUtils.writeStringToFile(actualExplainFile, explain,
StandardCharsets.UTF_8)
-
- fail(s"""
- |Plans did not match:
- |last approved simplified plan:
${approvedSimplifiedFile.getAbsolutePath}
- |last approved explain plan: ${approvedExplainFile.getAbsolutePath}
- |
- |$approvedSimplified
- |
- |actual simplified plan: ${actualSimplifiedFile.getAbsolutePath}
- |actual explain plan: ${actualExplainFile.getAbsolutePath}
- |
- |$actualSimplified
- """.stripMargin)
+ FileUtils.writeStringToFile(actualExplainFile, actualExplain,
StandardCharsets.UTF_8)
+
+ comparePlans(
+ "simplified",
+ approvedSimplified,
+ actualSimplified,
+ approvedSimplifiedFile,
+ actualSimplifiedFile)
+
+ comparePlans(
+ "explain",
+ approvedExplain,
+ actualExplain,
+ approvedExplainFile,
+ actualExplainFile)
+ }
+ }
+
+ private def comparePlans(
+ planType: String,
+ expected: String,
+ actual: String,
+ expectedFile: File,
+ actualFile: File): Unit = {
+ if (expected != actual) {
+ val message =
+ s"Expected $planType plan in ${expectedFile.getAbsolutePath} did not
match " +
+ s"actual $planType plan in ${actualFile.getAbsolutePath}"
+ throw new ComparisonFailure(message, expected, actual)
Review Comment:
Thanks. I reverted to the previous approach of calling `fail` with the
formatted plans
--
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]
