(Whereas today if you run that test w/o assertions you get a failure, albeit a confusing one).
Actually today when you run the tests - with assertions, without assertions, - you get no failures at all - which is what I was trying to fix here (unless I missed something seriously) - because:
- the original tests, after deciding to fail, invoked fail()
- this threw AssertionError
- but it was ignored as part of their wrong logic.
I'm confused here – the changes to TestSegmentMerger look like they'll allow the test to pass when assertions are disabled?
Right, I fixed it such that only if assertions are enabled, they verify that the expected assertion errors are not thrown, so they allow you to run tests also without enabling assertions. See my comment above "only one test...". I take it that this kind of flexibility is not required. So will change it so that these tests will fail if assertions are not enabled.
The other day I committed an accidental change to common-build that disabled assertions, and it was a little confusing to track down.
I see, so we make the entire test framework to fail if assertions are not enabled.
I'll update the patch.