venkata91 commented on a change in pull request #33425:
URL: https://github.com/apache/spark/pull/33425#discussion_r673340152



##########
File path: core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala
##########
@@ -2093,6 +2093,9 @@ class BlockManagerSuite extends SparkFunSuite with 
Matchers with BeforeAndAfterE
       Seq("hostC", "hostB", "hostD").sorted)
     assert(master.getShufflePushMergerLocations(4, 
Set.empty).map(_.host).sorted ===
       Seq("hostB", "hostA", "hostC", "hostD").sorted)
+    master.removeShufflePushMergerLocation("hostA")
+    assert(master.getShufflePushMergerLocations(4, 
Set.empty).map(_.host).sorted ===
+      Seq("hostB", "hostC", "hostD").sorted)

Review comment:
       I tried doing that, currently the BlockManagerMaster is mocked, 
therefore it doesn't register due to which it won't be added as part of 
`blockManagerIdByExecutor`. If not the test should have failed in the first 
place itself, as we are explicitly having tests checking for the hosts. But I 
will see if there is any other way add tests for this thing.




-- 
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]

Reply via email to