testtools-dev team mailing list archive

[Jonathan Lange <jml@xxxxxxxxxxxxx>]

Review: Needs Fixing

Thanks for the patch, Raphaël. 

I don't quite understand the matcher though.  From the docstring and the name, I would have guess something like this::

  self.assertThat([1, 2, 3], ContainsAll([3, 1]))

That is, self.assertThat(Y, ContainsAll(X)) would pass if and only if X is a subset of Y.

As it is, self.assertThat(Y, ContainsAll(X)) passes iff for all y in Y, for all x in X, x in y.  (i.e. ∀ y ∊ Y, ∀ x ∊ X, 'x in y')

So, to me, the name is confusing and my lazy imagination isn't leaping to actual use cases.

Where to from here?

 * Demonstrate use cases

 * Pick a better name, maybe EachContainsAll


Follow up work for testtools more generally:

 * some kind of more generic helper might be interesting, e.g.
     lambda matcher, items: MatchesAll(*[matcher(item) for item in items], first_only=False)
   I don't really know what it ought to be called. Sorry.

 * MatchesAll is really an "And"-style function.  It would probably be better if its name reflected that.

 * More generally, thinking a little bit more about predicate & set algebra might provide us with some better combinators


Thanks,
jml
-- 
https://code.launchpad.net/~rvb/testtools/testtools-contains-all/+merge/104065
Your team testtools developers is subscribed to branch lp:testtools.