[backport] bring AssertUtil fixes to 2.12 #9108
Conversation
| val seq2 = List("third") | ||
| val it0: Iterator[Int] = Iterator(1, 2) | ||
| lazy val it: Iterator[String] = it0.flatMap { | ||
| case 1 => val r = seq1; seq1 = null; seq11 = r(1); r |
There was a problem hiding this comment.
This was wrong. Setting seq1 = null here actually "breaks" subsequent check calls. They are intended to check whether the array is reachable from the iterator, passing null was undermining the check. This was not caught because of the buggy assertThrows.
Instead of coming up with my own fix, I took the version from 2.13.
|
I'll try to contribute a review when I have a free pomodoro. I agree with the comment about test tools. But it's never fun futzing with old branches. |
|
|
Backport of PR 9106. IteratorTest.`flatMap is memory efficient in previous element` was copied from 2.13, as the 2.12 version was wrong (started failing with the fixed `assertThrows`). The change in AssertThrowsTest is backported from 4eab5e0.
lrytz
force-pushed
the
assertUtilBackport
branch
from
d80c414 to
b3c0fc0
Compare
Backported the test fix from 4eab5e0679 |
|
I feel like I haven't slept since December 2018 when I promised to put 👀 on it later or 🙄 ... |
Previous version ought to have awaited the first completed of, but using the parasitic context should suffice; we check that with an assertion.
|
@som-snytt I included backports from #9112. |
som-snytt
left a comment
som-snytt
left a comment
There was a problem hiding this comment.
I'm tempted to try and fully understand the test. It must be right, right?
A good sign is that when I remove an unnecessary bit, the test fails.
SethTisue
added
the
internal
4 participants
Having buggy test tools is quite dangerous.
Backport of #9106, #9112