Some useful checkers.

Some useful checkers.

Also some testing functions used later, with their own test.

https://code.launchpad.net/~thumper/juju-core/moar-checkers/+merge/170227

(do not edit description out of merge proposal)

[thumper, 2013-06-19 00:20:53]

Please take a look.

[thumper, 2013-06-19 00:33:51]

Please take a look.

[wallyworld, 2013-06-19 00:50:23]

LGTM

https://codereview.appspot.com/10395044/diff/3001/testing/checkers/bool.go
File testing/checkers/bool.go (right):

https://codereview.appspot.com/10395044/diff/3001/testing/checkers/bool.go#ne...
testing/checkers/bool.go:27: return false, "obtained value not a bool"
If possible, also add the value and type to this error message, like for the
other greaterThan checker

[thumper, 2013-06-19 01:12:58]

Please take a look.

https://codereview.appspot.com/10395044/diff/3001/testing/checkers/bool.go
File testing/checkers/bool.go (right):

https://codereview.appspot.com/10395044/diff/3001/testing/checkers/bool.go#ne...
testing/checkers/bool.go:27: return false, "obtained value not a bool"
On 2013/06/19 00:50:23, wallyworld wrote:
> If possible, also add the value and type to this error message, like for the
> other greaterThan checker

Done.

[jameinel, 2013-06-19 04:32:28]

LGTM

I have some comments about it, but the only one I really want to see fixed is
s/recieved/received/.

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool_test.go
File testing/checkers/bool_test.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool_test....
testing/checkers/bool_test.go:22: }
'received' which I think means you need to update the checker as well.

Would it be better to have the message:

expected "true" (type bool), received string: "foo"

Or to make the two halves consistent:

`expected type bool "true", received string "foo"`

I don't think you need the colon and to quote the string. Though I'm soft on
that.

Mostly, I'd like that if we are going to explicitly list what we obtained, we
should explicitly list what we expected.

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test.go
File testing/checkers/relop_test.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test...
testing/checkers/relop_test.go:24: }
I wonder if spelling this GreaterThan is best. Mostly because I'm thinking of
the permutations:
GreaterThanOrEqualTo

Which is pretty commonly abbreviated GTE.

I'm also hoping the infrastructure can give us a nice "Between" (though again
you have to watch out for whether one or both ends are open or closed a < x < b
vs a <= x < b, etc)

https://codereview.appspot.com/10395044/diff/7001/testing/file.go
File testing/file.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/file.go#newcode17
testing/file.go:17: }
Couldn't this be done as a checker too?

c.Assert(filename, IsNonEmptyFile) ?

I'm fine with this helper, but checkers do tend to be slightly more consistent
with how we do assertions.

Also, it lets you use "c.Check" when appropriate, rather than always Assert
(fail the test but apply more checks vs fail immediately and stop).

[thumper, 2013-06-19 05:01:28]

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool_test.go
File testing/checkers/bool_test.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool_test....
testing/checkers/bool_test.go:22: }
On 2013/06/19 04:32:28, jameinel wrote:
> 'received' which I think means you need to update the checker as well.
> 
> Would it be better to have the message:
> 
> expected "true" (type bool), received string: "foo"
> 
> Or to make the two halves consistent:
> 
> `expected type bool "true", received string "foo"`
> 
> I don't think you need the colon and to quote the string. Though I'm soft on
> that.
> 
> Mostly, I'd like that if we are going to explicitly list what we obtained, we
> should explicitly list what we expected.

The quotes are part of %#v, so if we had 

  42, that would become: 
    int:42

Fixed received spelling, went for:

  `expected bool:true, received string:"foo"`
and added a test
  `expected bool:true, received int:42`

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test.go
File testing/checkers/relop_test.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test...
testing/checkers/relop_test.go:24: }
On 2013/06/19 04:32:28, jameinel wrote:
> I wonder if spelling this GreaterThan is best. Mostly because I'm thinking of
> the permutations:
> GreaterThanOrEqualTo
> 
> Which is pretty commonly abbreviated GTE.
> 
> I'm also hoping the infrastructure can give us a nice "Between" (though again
> you have to watch out for whether one or both ends are open or closed a < x <
b
> vs a <= x < b, etc)

python testtools uses GreaterThan too :)

Yes, you can do a between, and I do have one for time.

https://codereview.appspot.com/10395044/diff/7001/testing/file.go
File testing/file.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/file.go#newcode17
testing/file.go:17: }
On 2013/06/19 04:32:28, jameinel wrote:
> Couldn't this be done as a checker too?
> 
> c.Assert(filename, IsNonEmptyFile) ?
> 
> I'm fine with this helper, but checkers do tend to be slightly more consistent
> with how we do assertions.
> 
> Also, it lets you use "c.Check" when appropriate, rather than always Assert
> (fail the test but apply more checks vs fail immediately and stop).

Yes... although I had trouble thinking of it at first.

Will fix.

[mue, 2013-06-19 08:31:29]

LGTM, thanks for moving it into an own CL.

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool.go
File testing/checkers/bool.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool.go#ne...
testing/checkers/bool.go:13: // IsTrue checker
Not needed, for the other checkers too. But please add godoc comments for the
exported symbols like IsTrue or IsFalse.

[rog, 2013-06-19 11:19:05]

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool.go
File testing/checkers/bool.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/bool.go#ne...
testing/checkers/bool.go:34: return false, fmt.Sprintf("expected type bool,
recieved %s:%#v", value.Kind(), params[0])
return false, fmt.Sprintf("expected type bool, received %s", value.Type())

?
(gocheck itself will print the value for you - no need to print it again).

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop.go
File testing/checkers/relop.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop.go#n...
testing/checkers/relop.go:51: return false, fmt.Sprintf("obtained value %s:%#v
not supported", p0value.Kind(), params[0])
fmt.Sprintf("expected numeric type, received %T", p0value)

?

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test.go
File testing/checkers/relop_test.go (right):

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test...
testing/checkers/relop_test.go:4: package checkers_test
i'm not sure we gain much by having a separate test file for each kind of
checker. i'd put them all in one file and test suite (checkers_test.go), making
it easy to scan 'em all at once.

https://codereview.appspot.com/10395044/diff/7001/testing/checkers/relop_test...
testing/checkers/relop_test.go:8: . "launchpad.net/juju-core/testing/checkers"
I object to the spreading use of import to .

gocheck itself is bad enough.

Rather than polluting the local package name space,
could we choose a suitable short package identifier
or alias please? Or just use "checkers" itself.

if we adopted the fairly standard abbreviations
that john suggests, we could have something like:

c.Assert(45, checkers.GT, 42)

which isn't too much typing, and it's obvious where
the names come from.