Github user rezasafi commented on the issue:
https://github.com/apache/spark/pull/22325
it seems that it is safer to not merge the two exception handling, since
doing that will require moving around or removing some val that isn't clear why
are there. Merging and removing originalInput has caused the test "successful
3 local reads + 2 remote reads" in ShuffleBlockFetcherIteratorSuite to failed
by a message like:
`org.mockito.exceptions.verification.NeverWantedButInvoked:
inputStream.close();
Never wanted here:
-> at
org.apache.spark.storage.ShuffleBlockFetcherIteratorSuite$$anonfun$1$$anonfun$apply$mcV$sp$1.apply$mcVI$sp(ShuffleBlockFetcherIteratorSuite.scala:131)
But invoked here:
-> at
org.apache.spark.storage.ShuffleBlockFetcherIterator.next(ShuffleBlockFetcherIterator.scala:474)
at
org.apache.spark.storage.ShuffleBlockFetcherIteratorSuite$$anonfun$1$$anonfun$apply$mcV$sp$1.apply$mcVI$sp(ShuffleBlockFetcherIteratorSuite.scala:131)
at scala.collection.immutable.Range.foreach$mVc$sp(Range.scala:160)`
If we change the test in lines 131 to 137 by increasing the number of times
that inputStream.close() is expected to be called by one, the test will be
passed. I think that kind of make sense since we are moving "input" to the
inside of the try and will probably increase the number of expected close()
calls by one. However I'm not sure if I understand the purpose of that test
correctly and don't feel confident for that change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
