Merge lp:~rvb/testtools/testtools-contains-all into lp:~testtools-committers/testtools/trunk

[Raphaël Badin]

New matcher ContainsAll.

This branch adds a new (shortcut) matcher ContainsAll. ContainsAll(items) that can be used instead of using MatchesAll(*[Contains(item) for item in items])). We use it quite a lot in MAAS so we figured we could submit it upstream.

Drive-by fix: removed unused import.

[Jonathan Lange (jml)]

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

review: Needs Fixing

[Raphaël Badin (rvb)]

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

Indeed, that's precisely what this matcher does. Again, it's simply a shortcut, it uses existing functionality.

> 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')

Maybe I'm missing something but that's not what this matcher does. I can't resist using these nice utf8 characters:
self.assertThat(y, ContainsAll(X)) passes iff for all x in X, x in y. (i.e. ∀ x ∊ X, 'x in y')

[Jonathan Lange (jml)]

OK, so this does behave as like a subset matcher. Oops.

<jml> rvba: the problem was that I read this:
 + matches_matches = ['foobar', 'foozbar', 'bar foo']
 as this:
 + matches_matches = [['foobar', 'foozbar', 'bar foo']]

Raphaël, could you please update the tests to not rely on string containment -- I think that would be clearer. e.g.

 + matches_matches = [['foo', 'bar'], ['foo', 'z', 'bar'], ['bar', 'foo']]

Also if you could add another sentence to the class docstring using the word 'subset' somewhere that'd be great.

jml

[lp:~rvb/testtools/testtools-contains-all]

259. By Raphaël Badin on 2012-04-30

Refactor tests. Improve doc.

[Raphaël Badin (rvb)]

> Raphaël, could you please update the tests to not rely on string containment
> -- I think that would be clearer. e.g.
>
> + matches_matches = [['foo', 'bar'], ['foo', 'z', 'bar'], ['bar', 'foo']]

Sure, I've refactored matches_matches and matches_mismatches in TestContainsAllInterface to not rely on string containment.

> Also if you could add another sentence to the class docstring using the word
> 'subset' somewhere that'd be great.

Done. Feel free to suggest something else for the docstring if the one I put doesn't suit you.

[Raphaël Badin (rvb)]

Hi Jonathan,

If you think this is worth it, could you please have another look at this? I think I've made all the improvements you requested.

R.

=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2012-04-27 13:41:16 +0000
+++ testtools/matchers.py	2012-04-30 14:41:18 +0000
@@ -16,6 +16,7 @@
     'AllMatch',
     'Annotate',
     'Contains',
+    'ContainsAll',
     'DirExists',
     'DocTestMatches',
     'EndsWith',
@@ -664,6 +665,16 @@
         return None
 
 
+def ContainsAll(items):
+    """Make a matcher that checks whether a list of things is contained
+    in another thing.
+
+    The matcher effectively checks that the provided sequence is a subset of
+    the matchee.
+    """
+    return MatchesAll(*map(Contains, items), first_only=False)
+
+
 class StartsWith(Matcher):
     """Checks whether one string starts with another."""
 

=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2011-12-21 01:28:38 +0000
+++ testtools/tests/test_matchers.py	2012-04-30 14:41:18 +0000
@@ -12,7 +12,6 @@
 
 from testtools import (
     Matcher, # check that Matcher is exposed at the top level for docs.
-    skipIf,
     TestCase,
     )
 from testtools.compat import (
@@ -29,6 +28,7 @@
     AnnotatedMismatch,
     _BinaryMismatch,
     Contains,
+    ContainsAll,
     DirContains,
     DirExists,
     DocTestMatches,
@@ -409,6 +409,23 @@
     describe_examples = [("1 not in 2", 2, Contains(1))]
 
 
+class TestContainsAllInterface(TestCase, TestMatchersInterface):
+
+    matches_matcher = ContainsAll(['foo', 'bar'])
+    matches_matches = [['foo', 'bar'], ['foo', 'z', 'bar'], ['bar', 'foo']]
+    matches_mismatches = [['f', 'g'], ['foo', 'baz'], []]
+
+    str_examples = [(
+        "MatchesAll(Contains('foo'), Contains('bar'))",
+        ContainsAll(['foo', 'bar'])),
+        ]
+
+    describe_examples = [("""Differences: [
+'baz' not in 'foo'
+]""",
+    'foo', ContainsAll(['foo', 'baz']))]
+
+
 def make_error(type, *args, **kwargs):
     try:
         raise type(*args, **kwargs)