This is an automated email from the ASF dual-hosted git repository.
yangjie01 pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.5 by this push:
new 5ccd68b23581 [SPARK-52944][CORE][SQL][YARN][TESTS][3.5] Fix invalid
assertions in tests
5ccd68b23581 is described below
commit 5ccd68b23581915152726cd9ba9d9e5b619a77c9
Author: yangjie01 <[email protected]>
AuthorDate: Mon Jul 28 12:07:31 2025 +0800
[SPARK-52944][CORE][SQL][YARN][TESTS][3.5] Fix invalid assertions in tests
### What changes were proposed in this pull request?
This pr fixes some invalid assertions in the test code, mainly addressing
two types of issues:
1. Forgetting to use `assert`. For example:
https://github.com/apache/spark/blob/80c1f5f2caaaaf059178aa80cad4dfefe68bbebe/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala#L321-L322
Here, it is clearly intended to assert that
`q.lastProgress.sink.numOutputRows == 0L"`, but the use of `assert` was
forgotten, rendering it an invalid expression.
2. Incorrect line breaks in `should be` statements, causing one line of
code to be mistakenly treated as two unrelated lines. For example:
https://github.com/apache/spark/blob/5a9929c32ef8aafd275a3cf4797bb0ba9a6e61e2/core/src/test/scala/org/apache/spark/SortShuffleSuite.scala#L72-L73
Here, it is clearly intended to make an assertion similar to the following:
```
filesCreatedByShuffle.map(_.getName) should be Set("shuffle_0_0_0.data",
"shuffle_0_0_0.index")
```
However, due to the incorrect line break, it actually fails to function as
an assertion.
Meanwhile, after implementing the aforementioned fixes, this pr also
addresses and repairs the failing test cases within it.
### Why are the changes needed?
Fix invalid assertions in tests
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
- Pass Github Actions
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #51677 from LuciferYang/SPARK-52944-3.5.
Authored-by: yangjie01 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
---
.../scala/org/apache/spark/SortShuffleSuite.scala | 7 +--
.../org/apache/spark/deploy/SparkSubmitSuite.scala | 16 +++----
.../network/yarn/YarnShuffleServiceSuite.scala | 56 +++++++++++-----------
.../expressions/UnsafeRowConverterSuite.scala | 8 ++--
.../streaming/StreamingDeduplicationSuite.scala | 2 +-
5 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/core/src/test/scala/org/apache/spark/SortShuffleSuite.scala
b/core/src/test/scala/org/apache/spark/SortShuffleSuite.scala
index 571110784818..f0e41046ae48 100644
--- a/core/src/test/scala/org/apache/spark/SortShuffleSuite.scala
+++ b/core/src/test/scala/org/apache/spark/SortShuffleSuite.scala
@@ -26,7 +26,7 @@ import org.apache.commons.io.filefilter.TrueFileFilter
import org.scalatest.BeforeAndAfterAll
import org.scalatest.matchers.should.Matchers._
-import org.apache.spark.internal.config.SHUFFLE_MANAGER
+import org.apache.spark.internal.config.{SHUFFLE_CHECKSUM_ALGORITHM,
SHUFFLE_MANAGER}
import org.apache.spark.rdd.ShuffledRDD
import org.apache.spark.serializer.{JavaSerializer, KryoSerializer}
import org.apache.spark.shuffle.sort.SortShuffleManager
@@ -69,8 +69,9 @@ class SortShuffleSuite extends ShuffleSuite with
BeforeAndAfterAll {
shuffledRdd.count()
// Ensure that the shuffle actually created files that will need to be
cleaned up
val filesCreatedByShuffle = getAllFiles -- filesBeforeShuffle
- filesCreatedByShuffle.map(_.getName) should be
- Set("shuffle_0_0_0.data", "shuffle_0_0_0.index")
+ filesCreatedByShuffle.map(_.getName) should be(
+ Set("shuffle_0_0_0.data",
s"shuffle_0_0_0.checksum.${conf.get(SHUFFLE_CHECKSUM_ALGORITHM)}",
+ "shuffle_0_0_0.index"))
// Check that the cleanup actually removes the files
sc.env.blockManager.master.removeShuffle(0, blocking = true)
for (file <- filesCreatedByShuffle) {
diff --git a/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
b/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
index 8e2d6e6cf5ff..dd8bb3b96480 100644
--- a/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala
@@ -1095,14 +1095,14 @@ class SparkSubmitSuite
val appArgs = new SparkSubmitArguments(args)
val (_, _, conf, _) = submit.prepareSubmitEnvironment(appArgs)
- conf.get("spark.yarn.dist.jars").split(",").toSet should be
- (Set(jar1.toURI.toString, jar2.toURI.toString))
- conf.get("spark.yarn.dist.files").split(",").toSet should be
- (Set(file1.toURI.toString, file2.toURI.toString))
- conf.get("spark.yarn.dist.pyFiles").split(",").toSet should be
- (Set(pyFile1.getAbsolutePath, pyFile2.getAbsolutePath))
- conf.get("spark.yarn.dist.archives").split(",").toSet should be
- (Set(archive1.toURI.toString, archive2.toURI.toString))
+ conf.get("spark.yarn.dist.jars").split(",").toSet should be(
+ Set(jar1.toURI.toString, jar2.toURI.toString))
+ conf.get("spark.yarn.dist.files").split(",").toSet should be(
+ Set(file1.toURI.toString, file2.toURI.toString))
+ conf.get("spark.yarn.dist.pyFiles").split(",").toSet should be(
+ Set(pyFile1.toURI.toString, pyFile2.toURI.toString))
+ conf.get("spark.yarn.dist.archives").split(",").toSet should be(
+ Set(archive1.toURI.toString, archive2.toURI.toString))
}
}
}
diff --git
a/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala
b/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala
index 552cc98311e8..5c98785089ef 100644
---
a/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala
+++
b/resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala
@@ -616,35 +616,35 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
val mergeManager1DB =
ShuffleTestAccessor.mergeManagerLevelDB(mergeManager1)
ShuffleTestAccessor.recoveryFile(mergeManager1) should be (mergeMgrFile)
- ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0
- ShuffleTestAccessor.reloadAppShuffleInfo(
- mergeManager1, mergeManager1DB).size() equals 0
+ assert(ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals
0)
+ assert(ShuffleTestAccessor.reloadAppShuffleInfo(
+ mergeManager1, mergeManager1DB).size() equals 0)
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
var appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 1
+ assert(appShuffleInfoAfterReload.size() equals 1)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
mergeManager1.registerExecutor(app2Attempt1Id.toString,
mergedShuffleInfo2Attempt1)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 2
+ assert(appShuffleInfo.size() equals 2)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfo.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 2
+ assert(appShuffleInfoAfterReload.size() equals 2)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfoAfterReload.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
mergeManager1.registerExecutor(app3IdNoAttemptId.toString,
mergedShuffleInfo3NoAttemptId)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 3
+ assert(appShuffleInfo.size() equals 3)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfo.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
@@ -652,7 +652,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
app3IdNoAttemptId.toString).getAppPathsInfo should be
(appPathsInfo3NoAttempt)
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 3
+ assert(appShuffleInfoAfterReload.size() equals 3)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfoAfterReload.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
@@ -661,7 +661,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
mergeManager1.registerExecutor(app2Attempt2Id.toString,
mergedShuffleInfo2Attempt2)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 3
+ assert(appShuffleInfo.size() equals 3)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfo.get(
app2Attempt2Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt2)
@@ -669,7 +669,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
app3IdNoAttemptId.toString).getAppPathsInfo should be
(appPathsInfo3NoAttempt)
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 3
+ assert(appShuffleInfoAfterReload.size() equals 3)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfoAfterReload.get(
app2Attempt2Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt2)
@@ -678,14 +678,14 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
mergeManager1.applicationRemoved(app2Attempt2Id.toString, true)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 2
+ assert(appShuffleInfo.size() equals 2)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
assert(!appShuffleInfo.containsKey(app2Attempt2Id.toString))
appShuffleInfo.get(
app3IdNoAttemptId.toString).getAppPathsInfo should be
(appPathsInfo3NoAttempt)
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 2
+ assert(appShuffleInfoAfterReload.size() equals 2)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
assert(!appShuffleInfoAfterReload.containsKey(app2Attempt2Id.toString))
appShuffleInfoAfterReload.get(
@@ -725,9 +725,9 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
val mergeManager1DB =
ShuffleTestAccessor.mergeManagerLevelDB(mergeManager1)
ShuffleTestAccessor.recoveryFile(mergeManager1) should be (mergeMgrFile)
- ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0
- ShuffleTestAccessor.reloadAppShuffleInfo(
- mergeManager1, mergeManager1DB).size() equals 0
+ assert(ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals
0)
+ assert(ShuffleTestAccessor.reloadAppShuffleInfo(
+ mergeManager1, mergeManager1DB).size() equals 0)
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
mergeManager1.registerExecutor(app2Attempt1Id.toString,
mergedShuffleInfo2Attempt1)
@@ -737,7 +737,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 2
+ assert(appShuffleInfo.size() equals 2)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfo.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
@@ -745,7 +745,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
assert(!appShuffleInfo.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
var appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 2
+ assert(appShuffleInfoAfterReload.size() equals 2)
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfoAfterReload.get(
app2Attempt1Id.toString).getAppPathsInfo should be
(appPathsInfo2Attempt1)
@@ -765,12 +765,12 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
mergeManager1.applicationRemoved(app1Id.toString, true)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
assert(!appShuffleInfo.containsKey(app1Id.toString))
assert(appShuffleInfo.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 1
+ assert(appShuffleInfoAfterReload.size() equals 1)
assert(!appShuffleInfoAfterReload.containsKey(app1Id.toString))
assert(appShuffleInfoAfterReload.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
@@ -844,7 +844,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 2
+ assert(appShuffleInfo.size() equals 2)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
appShuffleInfo.get(
app2Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
@@ -867,20 +867,20 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
mergeManager1.applicationRemoved(app1Id.toString, true)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
assert(!appShuffleInfo.containsKey(app1Id.toString))
assert(appShuffleInfo.get(app2Id.toString).getShuffles.get(2).isFinalized)
// Clear the AppsShuffleInfo hashmap and reload the hashmap from DB
appShuffleInfoAfterReload =
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
- appShuffleInfoAfterReload.size() equals 1
+ assert(appShuffleInfoAfterReload.size() equals 1)
assert(!appShuffleInfoAfterReload.containsKey(app1Id.toString))
assert(appShuffleInfoAfterReload.get(app2Id.toString).getShuffles.get(2).isFinalized)
// Register application app1Id again and reload the DB again
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 2
+ assert(appShuffleInfo.size() equals 2)
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be
(appPathsInfo1)
assert(appShuffleInfo.get(app1Id.toString).getShuffles.isEmpty)
assert(appShuffleInfo.get(app2Id.toString).getShuffles.get(2).isFinalized)
@@ -924,7 +924,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
prepareAppShufflePartition(mergeManager1, partitionId1, 2, "4")
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
appShuffleInfo.get(
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt1)
assert(!appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -938,7 +938,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
appShuffleInfo.get(
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
assert(!appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -973,7 +973,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
val mergeManager3 =
s3.shuffleMergeManager.asInstanceOf[RemoteBlockPushResolver]
val mergeManager3DB =
ShuffleTestAccessor.mergeManagerLevelDB(mergeManager3)
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager3)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
appShuffleInfo.get(
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
assert(appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -1014,7 +1014,7 @@ abstract class YarnShuffleServiceSuite extends
SparkFunSuite with Matchers {
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1Attempt2)
val appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
- appShuffleInfo.size() equals 1
+ assert(appShuffleInfo.size() equals 1)
appShuffleInfo.get(
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala
index cbab8894cb55..865e63405f4d 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala
@@ -115,14 +115,14 @@ class UnsafeRowConverterSuite extends SparkFunSuite with
Matchers with PlanTestB
// Date is represented as Int in unsafeRow
assert(DateTimeUtils.toJavaDate(unsafeRow.getInt(2)) ===
Date.valueOf("1970-01-01"))
// Timestamp is represented as Long in unsafeRow
- DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be
- (Timestamp.valueOf("2015-05-08 08:10:25"))
+ DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be(
+ Timestamp.valueOf("2015-05-08 08:10:25"))
unsafeRow.setInt(2, DateTimeUtils.fromJavaDate(Date.valueOf("2015-06-22")))
assert(DateTimeUtils.toJavaDate(unsafeRow.getInt(2)) ===
Date.valueOf("2015-06-22"))
unsafeRow.setLong(3,
DateTimeUtils.fromJavaTimestamp(Timestamp.valueOf("2015-06-22 08:10:25")))
- DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be
- (Timestamp.valueOf("2015-06-22 08:10:25"))
+ DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be(
+ Timestamp.valueOf("2015-06-22 08:10:25"))
}
testBothCodegenAndInterpreted(
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala
index c69088589cc2..168f4f245270 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala
@@ -319,7 +319,7 @@ class StreamingDeduplicationSuite extends
StateStoreMetricsTest {
},
AssertOnQuery { q =>
eventually(timeout(streamingTimeout)) {
- q.lastProgress.sink.numOutputRows == 0L
+ assert(q.lastProgress.sink.numOutputRows == 0L)
true
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
