Skip to content

Commit ec16e6e

Browse files
committed
[SPARK-52944][CORE][SQL][YARN] 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/5a9929c32ef8aafd275a3cf4797bb0ba9a6e61e2/sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeSuite.scala#L342 Here, it is clearly intended to assert that `dt.simpleString equals "struct<c1:string>"`, 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 #51654 from LuciferYang/unused-expressions. Authored-by: yangjie01 <[email protected]> Signed-off-by: yangjie01 <[email protected]> (cherry picked from commit 90fd991) Signed-off-by: yangjie01 <[email protected]>
1 parent e21749d commit ec16e6e

File tree

6 files changed

+47
-47
lines changed

6 files changed

+47
-47
lines changed

core/src/test/scala/org/apache/spark/SortShuffleSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ class SortShuffleSuite extends ShuffleSuite with BeforeAndAfterAll {
6969
shuffledRdd.count()
7070
// Ensure that the shuffle actually created files that will need to be cleaned up
7171
val filesCreatedByShuffle = getAllFiles -- filesBeforeShuffle
72-
filesCreatedByShuffle.map(_.getName) should be
73-
Set("shuffle_0_0_0.data", "shuffle_0_0_0.index")
72+
filesCreatedByShuffle.map(_.getName) should be(
73+
Set("shuffle_0_0_0.data", "shuffle_0_0_0.checksum.ADLER32", "shuffle_0_0_0.index"))
7474
// Check that the cleanup actually removes the files
7575
sc.env.blockManager.master.removeShuffle(0, blocking = true)
7676
for (file <- filesCreatedByShuffle) {

core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,14 +1189,14 @@ class SparkSubmitSuite
11891189

11901190
val appArgs = new SparkSubmitArguments(args)
11911191
val (_, _, conf, _) = submit.prepareSubmitEnvironment(appArgs)
1192-
conf.get("spark.yarn.dist.jars").split(",").toSet should be
1193-
(Set(jar1.toURI.toString, jar2.toURI.toString))
1194-
conf.get("spark.yarn.dist.files").split(",").toSet should be
1195-
(Set(file1.toURI.toString, file2.toURI.toString))
1196-
conf.get("spark.yarn.dist.pyFiles").split(",").toSet should be
1197-
(Set(pyFile1.getAbsolutePath, pyFile2.getAbsolutePath))
1198-
conf.get("spark.yarn.dist.archives").split(",").toSet should be
1199-
(Set(archive1.toURI.toString, archive2.toURI.toString))
1192+
conf.get("spark.yarn.dist.jars").split(",").toSet should be(
1193+
Set(jar1.toURI.toString, jar2.toURI.toString))
1194+
conf.get("spark.yarn.dist.files").split(",").toSet should be(
1195+
Set(file1.toURI.toString, file2.toURI.toString))
1196+
conf.get("spark.yarn.dist.pyFiles").split(",").toSet should be(
1197+
Set(pyFile1.toURI.toString, pyFile2.toURI.toString))
1198+
conf.get("spark.yarn.dist.archives").split(",").toSet should be(
1199+
Set(archive1.toURI.toString, archive2.toURI.toString))
12001200
}
12011201
}
12021202
}

resource-managers/yarn/src/test/scala/org/apache/spark/network/yarn/YarnShuffleServiceSuite.scala

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -616,43 +616,43 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
616616
val mergeManager1DB = ShuffleTestAccessor.mergeManagerDB(mergeManager1)
617617
ShuffleTestAccessor.recoveryFile(mergeManager1) should be (mergeMgrFile)
618618

619-
ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0
620-
ShuffleTestAccessor.reloadAppShuffleInfo(
621-
mergeManager1, mergeManager1DB).size() equals 0
619+
assert(ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0)
620+
assert(ShuffleTestAccessor.reloadAppShuffleInfo(
621+
mergeManager1, mergeManager1DB).size() equals 0)
622622

623623
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
624624
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
625-
appShuffleInfo.size() equals 1
625+
assert(appShuffleInfo.size() equals 1)
626626
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
627627
var appShuffleInfoAfterReload =
628628
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
629-
appShuffleInfoAfterReload.size() equals 1
629+
assert(appShuffleInfoAfterReload.size() equals 1)
630630
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
631631

632632
mergeManager1.registerExecutor(app2Attempt1Id.toString, mergedShuffleInfo2Attempt1)
633633
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
634-
appShuffleInfo.size() equals 2
634+
assert(appShuffleInfo.size() equals 2)
635635
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
636636
appShuffleInfo.get(
637637
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
638638
appShuffleInfoAfterReload =
639639
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
640-
appShuffleInfoAfterReload.size() equals 2
640+
assert(appShuffleInfoAfterReload.size() equals 2)
641641
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
642642
appShuffleInfoAfterReload.get(
643643
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
644644

645645
mergeManager1.registerExecutor(app3IdNoAttemptId.toString, mergedShuffleInfo3NoAttemptId)
646646
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
647-
appShuffleInfo.size() equals 3
647+
assert(appShuffleInfo.size() equals 3)
648648
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
649649
appShuffleInfo.get(
650650
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
651651
appShuffleInfo.get(
652652
app3IdNoAttemptId.toString).getAppPathsInfo should be (appPathsInfo3NoAttempt)
653653
appShuffleInfoAfterReload =
654654
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
655-
appShuffleInfoAfterReload.size() equals 3
655+
assert(appShuffleInfoAfterReload.size() equals 3)
656656
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
657657
appShuffleInfoAfterReload.get(
658658
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
@@ -661,15 +661,15 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
661661

662662
mergeManager1.registerExecutor(app2Attempt2Id.toString, mergedShuffleInfo2Attempt2)
663663
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
664-
appShuffleInfo.size() equals 3
664+
assert(appShuffleInfo.size() equals 3)
665665
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
666666
appShuffleInfo.get(
667667
app2Attempt2Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt2)
668668
appShuffleInfo.get(
669669
app3IdNoAttemptId.toString).getAppPathsInfo should be (appPathsInfo3NoAttempt)
670670
appShuffleInfoAfterReload =
671671
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
672-
appShuffleInfoAfterReload.size() equals 3
672+
assert(appShuffleInfoAfterReload.size() equals 3)
673673
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
674674
appShuffleInfoAfterReload.get(
675675
app2Attempt2Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt2)
@@ -678,14 +678,14 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
678678

679679
mergeManager1.applicationRemoved(app2Attempt2Id.toString, true)
680680
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
681-
appShuffleInfo.size() equals 2
681+
assert(appShuffleInfo.size() equals 2)
682682
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
683683
assert(!appShuffleInfo.containsKey(app2Attempt2Id.toString))
684684
appShuffleInfo.get(
685685
app3IdNoAttemptId.toString).getAppPathsInfo should be (appPathsInfo3NoAttempt)
686686
appShuffleInfoAfterReload =
687687
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
688-
appShuffleInfoAfterReload.size() equals 2
688+
assert(appShuffleInfoAfterReload.size() equals 2)
689689
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
690690
assert(!appShuffleInfoAfterReload.containsKey(app2Attempt2Id.toString))
691691
appShuffleInfoAfterReload.get(
@@ -725,9 +725,9 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
725725
val mergeManager1DB = ShuffleTestAccessor.mergeManagerDB(mergeManager1)
726726
ShuffleTestAccessor.recoveryFile(mergeManager1) should be (mergeMgrFile)
727727

728-
ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0
729-
ShuffleTestAccessor.reloadAppShuffleInfo(
730-
mergeManager1, mergeManager1DB).size() equals 0
728+
assert(ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1).size() equals 0)
729+
assert(ShuffleTestAccessor.reloadAppShuffleInfo(
730+
mergeManager1, mergeManager1DB).size() equals 0)
731731

732732
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
733733
mergeManager1.registerExecutor(app2Attempt1Id.toString, mergedShuffleInfo2Attempt1)
@@ -737,15 +737,15 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
737737
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
738738

739739
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
740-
appShuffleInfo.size() equals 2
740+
assert(appShuffleInfo.size() equals 2)
741741
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
742742
appShuffleInfo.get(
743743
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
744744
assert(!appShuffleInfo.get(app1Id.toString).getShuffles.get(1).isFinalized)
745745
assert(!appShuffleInfo.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
746746
var appShuffleInfoAfterReload =
747747
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
748-
appShuffleInfoAfterReload.size() equals 2
748+
assert(appShuffleInfoAfterReload.size() equals 2)
749749
appShuffleInfoAfterReload.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
750750
appShuffleInfoAfterReload.get(
751751
app2Attempt1Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
@@ -765,12 +765,12 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
765765

766766
mergeManager1.applicationRemoved(app1Id.toString, true)
767767
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
768-
appShuffleInfo.size() equals 1
768+
assert(appShuffleInfo.size() equals 1)
769769
assert(!appShuffleInfo.containsKey(app1Id.toString))
770770
assert(appShuffleInfo.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
771771
appShuffleInfoAfterReload =
772772
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
773-
appShuffleInfoAfterReload.size() equals 1
773+
assert(appShuffleInfoAfterReload.size() equals 1)
774774
assert(!appShuffleInfoAfterReload.containsKey(app1Id.toString))
775775
assert(appShuffleInfoAfterReload.get(app2Attempt1Id.toString).getShuffles.get(2).isFinalized)
776776

@@ -844,7 +844,7 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
844844
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
845845

846846
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
847-
appShuffleInfo.size() equals 2
847+
assert(appShuffleInfo.size() equals 2)
848848
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
849849
appShuffleInfo.get(
850850
app2Id.toString).getAppPathsInfo should be (appPathsInfo2Attempt1)
@@ -867,20 +867,20 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
867867
mergeManager1.applicationRemoved(app1Id.toString, true)
868868

869869
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
870-
appShuffleInfo.size() equals 1
870+
assert(appShuffleInfo.size() equals 1)
871871
assert(!appShuffleInfo.containsKey(app1Id.toString))
872872
assert(appShuffleInfo.get(app2Id.toString).getShuffles.get(2).isFinalized)
873873
// Clear the AppsShuffleInfo hashmap and reload the hashmap from DB
874874
appShuffleInfoAfterReload =
875875
ShuffleTestAccessor.reloadAppShuffleInfo(mergeManager1, mergeManager1DB)
876-
appShuffleInfoAfterReload.size() equals 1
876+
assert(appShuffleInfoAfterReload.size() equals 1)
877877
assert(!appShuffleInfoAfterReload.containsKey(app1Id.toString))
878878
assert(appShuffleInfoAfterReload.get(app2Id.toString).getShuffles.get(2).isFinalized)
879879

880880
// Register application app1Id again and reload the DB again
881881
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1)
882882
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
883-
appShuffleInfo.size() equals 2
883+
assert(appShuffleInfo.size() equals 2)
884884
appShuffleInfo.get(app1Id.toString).getAppPathsInfo should be (appPathsInfo1)
885885
assert(appShuffleInfo.get(app1Id.toString).getShuffles.isEmpty)
886886
assert(appShuffleInfo.get(app2Id.toString).getShuffles.get(2).isFinalized)
@@ -924,7 +924,7 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
924924
prepareAppShufflePartition(mergeManager1, partitionId1, 2, "4")
925925

926926
var appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
927-
appShuffleInfo.size() equals 1
927+
assert(appShuffleInfo.size() equals 1)
928928
appShuffleInfo.get(
929929
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt1)
930930
assert(!appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -938,7 +938,7 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
938938
prepareAppShufflePartition(mergeManager1, partitionId2, 2, "4")
939939

940940
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
941-
appShuffleInfo.size() equals 1
941+
assert(appShuffleInfo.size() equals 1)
942942
appShuffleInfo.get(
943943
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
944944
assert(!appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -973,7 +973,7 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
973973
val mergeManager3 = s3.shuffleMergeManager.asInstanceOf[RemoteBlockPushResolver]
974974
val mergeManager3DB = ShuffleTestAccessor.mergeManagerDB(mergeManager3)
975975
appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager3)
976-
appShuffleInfo.size() equals 1
976+
assert(appShuffleInfo.size() equals 1)
977977
appShuffleInfo.get(
978978
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
979979
assert(appShuffleInfo.get(app1Id.toString).getShuffles.get(2).isFinalized)
@@ -1014,7 +1014,7 @@ abstract class YarnShuffleServiceSuite extends SparkFunSuite with Matchers {
10141014
mergeManager1.registerExecutor(app1Id.toString, mergedShuffleInfo1Attempt2)
10151015

10161016
val appShuffleInfo = ShuffleTestAccessor.getAppsShuffleInfo(mergeManager1)
1017-
appShuffleInfo.size() equals 1
1017+
assert(appShuffleInfo.size() equals 1)
10181018
appShuffleInfo.get(
10191019
app1Id.toString).getAppPathsInfo should be (appPathsInfo1Attempt2)
10201020

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,14 @@ class UnsafeRowConverterSuite extends SparkFunSuite with Matchers with Expressio
114114
// Date is represented as Int in unsafeRow
115115
assert(DateTimeUtils.toJavaDate(unsafeRow.getInt(2)) === Date.valueOf("1970-01-01"))
116116
// Timestamp is represented as Long in unsafeRow
117-
DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be
118-
(Timestamp.valueOf("2015-05-08 08:10:25"))
117+
DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be(
118+
Timestamp.valueOf("2015-05-08 08:10:25"))
119119

120120
unsafeRow.setInt(2, DateTimeUtils.fromJavaDate(Date.valueOf("2015-06-22")))
121121
assert(DateTimeUtils.toJavaDate(unsafeRow.getInt(2)) === Date.valueOf("2015-06-22"))
122122
unsafeRow.setLong(3, DateTimeUtils.fromJavaTimestamp(Timestamp.valueOf("2015-06-22 08:10:25")))
123-
DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be
124-
(Timestamp.valueOf("2015-06-22 08:10:25"))
123+
DateTimeUtils.toJavaTimestamp(unsafeRow.getLong(3)) should be(
124+
Timestamp.valueOf("2015-06-22 08:10:25"))
125125
}
126126

127127
testBothCodegenAndInterpreted(

sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeSuite.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -339,11 +339,11 @@ class DataTypeSuite extends SparkFunSuite {
339339
|""".stripMargin
340340
val dt = DataType.fromJson(schema)
341341

342-
dt.simpleString equals "struct<c1:string>"
343-
dt.json equals
342+
assert(dt.simpleString equals "struct<c1:string>")
343+
assert(dt.json equals
344344
"""
345-
|{"type":"struct","fields":[{"name":"c1","type":"string","nullable":false,"metadata":{}}]}
346-
|""".stripMargin
345+
|{"type":"struct","fields":[{"name":"c1","type":"string","nullable":true,"metadata":{}}]}
346+
|""".stripMargin.trim)
347347
}
348348

349349
def checkDefaultSize(dataType: DataType, expectedDefaultSize: Int): Unit = {

sql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingDeduplicationSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ class StreamingDeduplicationSuite extends StateStoreMetricsTest {
321321
},
322322
AssertOnQuery { q =>
323323
eventually(timeout(streamingTimeout)) {
324-
q.lastProgress.sink.numOutputRows == 0L
324+
assert(q.lastProgress.sink.numOutputRows == 0L)
325325
true
326326
}
327327
}

0 commit comments

Comments
 (0)