Merge lp:~allenap/testtools/lessthan-mismatch-string into lp:~testtools-committers/testtools/trunk

[Gavin Panella]

The other _BinaryComparison subclasses - Equals, NotEquals, Is - give
mismatch messages that describe the problem. LessThan gives a mismatch
message that appears to describe the test... except that it doesn't.

Presently it'll say things like:

   44 is >= 55
   12 is >= 12

I've changed it to say:

   44 is not > 55
   12 is not > 12

[Robert Collins (lifeless)]

This is inverted I think. Adding another test for assertThat(4.1,
LessThan(4.0)) should show this more clearly.

I can understand the current warning being confusing, but I'm pretty
sure it is correct.

[Gavin Panella (allenap)]

Mmm, I'm not sure:

  >>> LessThan(4.0).match(4.1).describe()
  '4.0 is >= 4.0999999999999996'

I think that a description of the test would be '4.0 is > 4.1', and a
description of the outcome would be '4.0 is not > 4.1'.

[Robert Collins (lifeless)]

 Wed, Apr 13, 2011 at 8:23 PM, Gavin Panella
<email address hidden> wrote:
> Mmm, I'm not sure:
>
>  >>> LessThan(4.0).match(4.1).describe()
>  '4.0 is >= 4.0999999999999996'
>
> I think that a description of the test would be '4.0 is > 4.1', and a
> description of the outcome would be '4.0 is not > 4.1'.

The output that was printed is clearly bong (and thats a bug). It
should say '<'.
'not >' would be wrong, because == is permitted.

Basically the output arguments are inverted - the result of match is a
*mis*match.
So 'it is not true that '4.1' (the thing being matched') is < '4.0'
(the thing describing what can match).
->
Is is true that '4.1'(the thing being matched) is >= '4.0' (the thing
describing what can match).
(which is still correct).

But we're putting the thing which can match first, so we need to invert it all.

Anyhow - I think we need a second test still, and the change should be
from >= to <, without adding the word not. (or just add the word not)

[Gavin Panella (allenap)]

> The output that was printed is clearly bong (and thats a bug). It
> should say '<'. 'not >' would be wrong, because == is permitted.

The matcher is LessThan, so == should not be permitted.

The other _BinaryComparison subclasses describe the failed state. Spot
the odd one out below:

  >>> matchers.Equals(1).match(2).describe()
  '1 != 2'
  >>> matchers.NotEquals(1).match(1).describe()
  '1 == 1'
  >>> matchers.Is(1).match(2).describe()
  '1 is not 2'
  >>> matchers.LessThan(1).match(2).describe()
  '1 is >= 2'

If the arguments appear in the mismatch message in the same order as
Equals, NotEquals and Is - namely expected followed by observed - then
the LessThan mismatch message should say either:

  >>> matchers.LessThan(1).match(2).describe()
  '1 is <= 2'

or:

  >>> matchers.LessThan(1).match(2).describe()
  '1 is not > 2'

However, a better description might be:

  >>> matchers.LessThan(1).match(2).describe()
  '2 is not < 1'

or even:

  >>> matchers.LessThan(1).match(2).describe()
  '2 is not less than 1'

[lp:~allenap/testtools/lessthan-mismatch-string]

186. By Gavin Panella on 2011-04-14

Another test for LessThan mismatch descriptions that reveals the order in which the expected and observed values appear.

[Jonathan Lange (jml)]

To summarize::

 * In trunk, if LessThan(small).match(big) fails, the mismatch says 'small is >= big', which is untrue
 * This should say 'big is >= small' OR 'small is not < big', because those are true facts about the bad state of affairs

Gavin's patch makes it say 'small is not > big'. I think this is just a thinko, reading '>' as 'less than' when it is actually 'greater than'.

There's some discussion above that talks about how the complement of '>' is '<=' (or '<' and '>=', it's easy to get lost). That is, 'not x > y' is equivalent to 'x <= y' and 'not x < y' is equivalent to 'x >= y'. I think we all get this.

[Jonathan Lange (jml)]

Ugh. Confused again. I really find this stuff hard. Sorry.

 * A passing test would have LessThan(big).match(small)
 * A failing test would have LessThan(small).match(too_big)
 * This could be validly expressed as 'too_big >= small' or 'small not < too_big'
 * The latter is easier to do with testtools as it stands.

[Jonathan Lange (jml)]

OK. This time for sure.

assertThat(too_big, LessThan(small)) fails.

You can express this as:
 A. not (too_big < small); too_big not < small
 B. too_big >= small
 C. small <= too_big
 D. not (small > too_big); small not > too_big

I defy anyone to disagree.

Gavin's patch does 'D', which is correct.

Trunk does 'small >= too_big', which is probably a well-intentioned but wrong version of B.

The proper fix for this bug would put 'too_big' in front (actually I think putting 'observed' in front might make sense in general -- another conversation). However, Gavin's fix is both correct and an incremental improvement.

review: Approve

=== modified file 'testtools/matchers.py'
--- testtools/matchers.py	2011-04-01 14:09:24 +0000
+++ testtools/matchers.py	2011-04-14 10:32:33 +0000
@@ -290,7 +290,7 @@
     """Matches if the item is less than the matchers reference object."""
 
     comparator = operator.__lt__
-    mismatch_string = 'is >='
+    mismatch_string = 'is not >'
 
 
 class MatchesAny(object):

=== modified file 'testtools/tests/test_matchers.py'
--- testtools/tests/test_matchers.py	2011-04-01 14:09:24 +0000
+++ testtools/tests/test_matchers.py	2011-04-14 10:32:33 +0000
@@ -168,7 +168,10 @@
         ("LessThan(12)", LessThan(12)),
         ]
 
-    describe_examples = [('4 is >= 4', 4, LessThan(4))]
+    describe_examples = [
+        ('4 is not > 5', 5, LessThan(4)),
+        ('4 is not > 4', 4, LessThan(4)),
+        ]
 
 
 def make_error(type, *args, **kwargs):