Code review comment for lp:~lifeless/testtools/matchers

On Mon, Aug 15, 2011 at 1:01 PM, Robert Collins
<email address hidden> wrote:
> On Mon, Aug 15, 2011 at 11:44 PM, Jonathan Lange <email address hidden> wrote:
>> Review: Needs Fixing
>> Thanks for doing this.
>>
>> doc/for-test-authors.rst:
>>  * The heading for the IsInstance matcher is "Is", should be "IsInstance".
>>
>> testtools/testcase.py
>>  * Please delete the XXX in assertThat about taking an optional message parameter.
>>  * The line "raise matchee[0], matchee[1], matchee[2]" is invalid syntax in Python 3. Use testtools.compat.reraise(*matchee) instead.
>
> The rest seem shallow; I will get to them at some point(ETOOMUCHON),
> perhaps you would like to just fix-as-landing ?
>

Am happy to do so.

>>  * Is there a reason that expectFailure wasn't also changed?
>
> flippantly, it didn't have a TODO; more seriously, it was complex, and
> the branch was big enough, and ugly enough, that I wanted to wrap it
> up.
>

Fair enough.

>> Generally, the new failure messages for the negative assert methods (e.g. assertNotIn) are worse than the current ones. To some extent, this would be addressed by a fix for bug 704219.
>
> What should we do about this?
>

Fix the bug, later.

jml