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

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 ?

>  * 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.

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

> Separately from this branch, I think we want to provide an In or IsIn matcher to complement Contains.

Yes.

-Rob