heyihong commented on code in PR #52073:
URL: https://github.com/apache/spark/pull/52073#discussion_r2299177694
##########
sql/core/src/test/scala/org/apache/spark/sql/artifact/ArtifactManagerSuite.scala:
##########
@@ -346,6 +347,71 @@ class ArtifactManagerSuite extends SharedSparkSession {
}
}
+ test("Add multiple artifacts to local session and check if all are added
despite exception") {
+ val copyDir = Utils.createTempDir().toPath
+ Utils.copyDirectory(artifactPath.toFile, copyDir.toFile)
+
+ val artifact1Path = "my/custom/pkg/artifact1.jar"
+ val artifact2Path = "my/custom/pkg/artifact2.jar"
+ val targetPath = Paths.get(artifact1Path)
+ val targetPath2 = Paths.get(artifact2Path)
+
+ val classPath1 = copyDir.resolve("Hello.class")
+ val classPath2 = copyDir.resolve("udf_noA.jar")
+ assume(artifactPath.resolve("Hello.class").toFile.exists)
+ assume(artifactPath.resolve("smallClassFile.class").toFile.exists)
+
+ val artifact1 = Artifact.newArtifactFromExtension(
+ targetPath.getFileName.toString,
+ targetPath,
+ new Artifact.LocalFile(Paths.get(classPath1.toString)))
+
+ val alreadyExistingArtifact = Artifact.newArtifactFromExtension(
+ targetPath2.getFileName.toString,
+ targetPath,
+ new Artifact.LocalFile(Paths.get(classPath2.toString)))
+
+ val artifact2 = Artifact.newArtifactFromExtension(
+ targetPath2.getFileName.toString,
+ targetPath2,
+ new Artifact.LocalFile(Paths.get(classPath2.toString)))
+
+ spark.artifactManager.addLocalArtifacts(Seq(artifact1))
+
+ val exception = intercept[SparkRuntimeException] {
+ spark.artifactManager.addLocalArtifacts(
+ Seq(alreadyExistingArtifact, artifact2, alreadyExistingArtifact))
+ }
+
+ // Validate exception: Should be ARTIFACT_ALREADY_EXISTS and have one
suppressed exception
+ assert(exception.getCondition == "ARTIFACT_ALREADY_EXISTS",
Review Comment:
It may be better practice to use checkError, as it also allows you to check
parameters. For example:
```scala
checkError(
exception = intercept[SparkIllegalArgumentException] {
ser.write(kryo, ks.newKryoOutput(), DefaultCachedBatch(1, null,
InternalRow.empty))
},
condition = "INVALID_KRYO_SERIALIZER_NO_DATA",
parameters = Map(
"obj" -> "DefaultCachedBatch.buffers",
"serdeOp" -> "serialize",
"serdeClass" -> ser.getClass.getName))
```
--
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-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
