Make matchers composable more easily

[thomir]

We love testtools matchers. We typically build some pretty complex matchers, which make our tests much easier to read. However, we frequently run into issues with the fact that re-using one matcher to implement a second one is tricky. At best it involves writing something like this inside the match function:

def match(self, matchee):
    mismatch = SomeOtherMatcher('foo').match(matchee)
    if mismatch:
        return mismatch

    mismatch = YetAnotherMatcher(123).match(matchee)
    if mismatch:
        return mismatch

   # etc.

...this gets tiresome quickly.

Another thing that sometimes happens is engineers try and work around this with subclassing one matcher from another:

class MyMatcher(AnotherMatcher):

    def match(self, matchee):
        mismatch = super().match(matchee)
        if mismatch: 
            return mismatch
        # check more things here.

...however this leads to all the classic issues surrounding using inheritance to re-use behavior.

I propose instead that the base Matcher class should contain an assertThat method, so the first example above could be more natually expressed as:

def match(self, matchee):
    self.assertThat(matchee, SomeOtherMatcher('foo'))
    self.assertThat(matchee, YetAnotherMatcher(123))
   # etc.

I'm filing this bug in order to solicit ideas/feedback - I can probably find some time to implement this myself in the next few weeks.

[jml]

Hi! I did a lot of work on this a year or so ago.

For ideas, see:

• https://gist.github.com/jml/96480de504578e1dd783
• https://lists.launchpad.net/testtools-dev/msg01217.html
• https://gist.github.com/jml/3a30695b6c22d98f1be0

For code, see:

• https://github.com/testing-cabal/testtools/pull/211, which was itself a re-implementation of
• https://github.com/testing-cabal/testtools/pull/181, which was a subset of what
• https://github.com/testing-cabal/testtools/pull/178 intended to do

where I tried to reduce inheritance and replace with interfaces. It's probably not necessary to make matcher more composable, but it's how I'd do it. I abandoned the effort after 8 months, due to not being able to sell the idea to the rest of the team and due to a 5 month wait for a re-review on #211.

[freeekanayaka]

@thomir can't the specific cases you mention be solved with MatchesAll (and possibly MatchesAny)? E.g.:

from testtools.matchers import MatchesAll

def MyMatcher(matchee):
    return MatchesAll(SomeOtherMatcher('foo'), YetAnotherMatcher(123))

while I agree that matchers composition needs a bit of love, I think that the testtools.matchers._higherorder package can already get you covered in several situations.

[freeekanayaka]

@jml do you think that there was actual disagreement on your ideas or just lack of review force? I gave a look at #211 and seems basically fine to me. I'll comment there. I also think you are on something with what you wrote in the gist page.

[jml]

IIRC, @rbtcollins was open to the idea of explicit interfaces as long as they were plain old Python classes (no abc or zope.interface, for reasons that elude me), and strongly against adapters.

I'm not sure everyone was sold on the general idea of reducing code-sharing inheritance by exposing only interfaces and functions.

[jml]

Worth noting: I had lots of follow-up done for #211 that I've since (probably) lost, or at least have only in hard-to-access archives. Removed most Mismatch subclasses and IIRC most Matcher subclasses. I'm not going to do this work again, but someone might want to.

[freeekanayaka]

Jonathan Lange notifications@github.com writes:

IIRC, @rbtcollins was open to the idea of explicit interfaces as long as they were plain old Python classes (no abc or zope.interface, for reasons that elude me), and strongly against adapters.

I'd be against adapters as public interface too for this specific case (expecially if they'd use the global registry). We might want to use adapters internally as implementation detail, but it feels really overkill.

I wouldn't oppose to zope.interface, although it brings in a slightly questionable dependency.

@rbcollins feedback is appreciated! :)

I'm not sure everyone was sold on the general idea of reducing code-sharing inheritance by exposing only interfaces and functions.

FWIW I'm sold on that part.

[thomir]

Hi @freeekanayaka - thanks for your reply.

MatchesAll certainly does improve the situation. However, it has a number of disadvantages (as noted in #227 - which I want to finish at some point), and to my mind doesn't feel particularly natural to compose matchers in this way - but I admit that's entirely subjective :)

MatchesAll also does not allow you to customise the details set by the matcher, and I can forsee cases where one might want to do exactly that. I'll try with MatchesAll and see where I get to.

Based on the conversation here, I suspect there's appetite for doing something to make matchers more composable.

[freeekanayaka]

@thomir yes, I agree that it does not feel particularly natural, and it has the limitations that you point. So MatchesAll is something you can use right now to alleviate the particular issues reported in this ticket, but let's leave this open for future reference in case somebody wants to tackle the problem appropriately.

[rbtcollins]

@jml - I'm open and pro explicit typing here - e.g. using the typing module and mypy would be great. zope.interfaces with the global registry of type adapters and so on I don't like: its not that it can't work (it obviously can), but its not something that is obvious or predictable to Python developers per se (unilke e.g. in rust where interface adaption is a commonly used idiom that is core to the language.; ABC brings runtime overheads but would be ok.

I'm strongly pro composition as a preferred approach for building rich matchers; I had specific code review concerns with the POC that was put up previously, from memory.

I'm disinterested in reducing the code we've got right now - I'll happily review patches to do it, but its not something I feel an urge to do myself. Making it easier for other folk to write matchers is something I think is important, and our existing code may be a good place to test out ideas for that.

I'm unsatisfied with our structural matching support, but not sure how to make it better right now ;(.

I don't recall how/why #211 stalled - it looks to me like it had been fully reviewed.

HTH. Rob