[GitHub] [spark] Hisoka-X commented on a diff in pull request #42952: [SPARK-45184][SQL] Remove orphaned error class documents

["Hisoka-X (via GitHub)" <gi...@apache.org>]

Hisoka-X commented on code in PR #42952:
URL: https://github.com/apache/spark/pull/42952#discussion_r1327921647


##########
core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:
##########
@@ -222,6 +223,21 @@ class SparkThrowableSuite extends SparkFunSuite {
          |---""".stripMargin
     }
 
+    def orphanedGoldenFiles(): Iterable[File] = {
+      val errors = errorReader.errorInfoMap

Review Comment:
   nit: in fact `errors` already be created in https://github.com/apache/spark/pull/42952/files#diff-b3bc05fec45cd951053b2876c71c7730b63789cb4336a7537a6654c724db3241R154



##########
core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:
##########
@@ -323,6 +339,23 @@ class SparkThrowableSuite extends SparkFunSuite {
       assert(sqlErrorParentDoc.trim == commonErrorsInDoc.trim,
         "The error class document is not up to date. Please regenerate it.")
     }
+
+    val orphans = orphanedGoldenFiles()
+    if (regenerateGoldenFiles) {
+      if (orphans.nonEmpty) {
+        logInfo(s"Orphaned error class documents (${orphans.size}) is not empty, " +
+          s"executing cleanup operation.")
+        orphans.foreach { f =>
+          FileUtils.deleteQuietly(f)
+          logInfo(s"Cleanup orphaned error document: ${f.getName}.")
+        }
+      } else {
+        logInfo(s"Orphaned error class documents is empty")
+      }
+    } else {
+      assert(orphans.isEmpty,
+        "Exist orphaned error class documents. Please regenerate it.")

Review Comment:
   Maybe we can add `orphans` value in error msg. Then developers can also just delete it by hands.



##########
core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:
##########
@@ -222,6 +223,21 @@ class SparkThrowableSuite extends SparkFunSuite {
          |---""".stripMargin
     }
 
+    def orphanedGoldenFiles(): Iterable[File] = {
+      val errors = errorReader.errorInfoMap
+      val subErrorFileNames = errors.filter(_._2.subClass.isDefined).map(error => {
+        getErrorPath(error._1) + ".md"
+      }).toSet
+
+      val docsDir = getWorkspaceFilePath("docs")
+      val orphans = FileUtils.listFiles(docsDir.toFile, Array("md"), false).filter { f =>
+        (f.getName.startsWith("sql-error-conditions-") && f.getName.endsWith("-error-class.md")) &&
+          !f.getName.equals("sql-error-conditions-sqlstates.md") &&

Review Comment:
   ```suggestion
   ```
   I believe if `(f.getName.startsWith("sql-error-conditions-") && f.getName.endsWith("-error-class.md"))` is true, this condition is also true.



##########
core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:
##########
@@ -323,6 +339,23 @@ class SparkThrowableSuite extends SparkFunSuite {
       assert(sqlErrorParentDoc.trim == commonErrorsInDoc.trim,
         "The error class document is not up to date. Please regenerate it.")
     }
+
+    val orphans = orphanedGoldenFiles()
+    if (regenerateGoldenFiles) {
+      if (orphans.nonEmpty) {
+        logInfo(s"Orphaned error class documents (${orphans.size}) is not empty, " +
+          s"executing cleanup operation.")
+        orphans.foreach { f =>
+          FileUtils.deleteQuietly(f)
+          logInfo(s"Cleanup orphaned error document: ${f.getName}.")
+        }
+      } else {
+        logInfo(s"Orphaned error class documents is empty")

Review Comment:
   ```suggestion
           logInfo("Orphaned error class documents is empty")
   ```



##########
core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:
##########
@@ -323,6 +339,23 @@ class SparkThrowableSuite extends SparkFunSuite {
       assert(sqlErrorParentDoc.trim == commonErrorsInDoc.trim,
         "The error class document is not up to date. Please regenerate it.")
     }
+
+    val orphans = orphanedGoldenFiles()
+    if (regenerateGoldenFiles) {
+      if (orphans.nonEmpty) {
+        logInfo(s"Orphaned error class documents (${orphans.size}) is not empty, " +
+          s"executing cleanup operation.")

Review Comment:
   ```suggestion
             "executing cleanup operation.")
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org