gfredericks / test.chuck

A utility library for test.check
Eclipse Public License 1.0
215 stars 26 forks source link

Checking macro prints not-falsey-or-exception? when fails #17

Closed firesofmay closed 8 years ago

firesofmay commented 9 years ago

Hi,

I was trying out checking macro and I found the output a little weird. Here's a sample test:

(deftest do-fail
  (checking "fail me" 1000
            [n gen/nat]
            (is (not= 10 n))))

And here's the output:

> (do-fail)

FAIL in (do-fail) (clojure_test.clj:17)
fail me
{:result false, :seed 1439064390947, :failing-size 48, :num-tests 49, :fail [10], :shrunk {:total-nodes-visited 4, :depth 0, :result false, :smallest [10]}}
expected: (not-falsey-or-exception? (:result result))
  actual: (not (not-falsey-or-exception? false))

FAIL in (do-fail) (demo.clj:32)
fail me
expected: (not= 10 n)
  actual: (not (not= 10 10))

Why does it print this?

expected: (not-falsey-or-exception? (:result result))
actual: (not (not-falsey-or-exception? false))
lackita commented 9 years ago

Looking into this now.

gfredericks commented 9 years ago

hey @lackita would the implementation for checking be easier if test.check provided callbacks for all the different events of the test-running process?

lackita commented 9 years ago

@gfredericks Maybe a little bit, the code currently assumes that the reports from the final failure are the smallest example, but I think I remember noticing minor discrepancies in practice. I might be able to eliminate those if I had a callbacks around successes, failures, and done shrinking.

gfredericks commented 9 years ago

Okay cool; I was working on such a thing at one point, so might be able to get it into the 0.9.0 release.

lackita commented 9 years ago

Alright, the specific trigger I'd find useful is one fired when a smaller example of a failure is identified. On Aug 10, 2015 8:35 AM, "Gary Fredericks" notifications@github.com wrote:

Okay cool; I was working on such a thing at one point, so might be able to get it into the 0.9.0 release.

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-129427866 .

gfredericks commented 9 years ago

I'm curious if that's because you would print something on each smaller failure?

gfredericks commented 9 years ago

I'm wondering because I've done such a thing myself in a test.check fork, and it ends up being very verbose, which makes me want to have a way to control verbosity.

lackita commented 9 years ago

Actually, quite the opposite. I only want to report the smallest failure and I need a way of determining if this new failure is the new smallest. On Aug 10, 2015 11:14 AM, "Gary Fredericks" notifications@github.com wrote:

I'm wondering because I've done such a thing myself in a test.check fork, and it ends up being very verbose, which makes me want to have a way to control verbosity.

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-129487995 .

gfredericks commented 9 years ago

why can't you do that just by waiting for the whole quick-check call to return and just examining the :shrunk key?

lackita commented 9 years ago

I wanted to present the actual failure report from those values, not the value leading to the failure. On Aug 10, 2015 12:59 PM, "Gary Fredericks" notifications@github.com wrote:

why can't you do that just by waiting for the whole quick-check call to return and just examining the :shrunk key?

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-129526344 .

gfredericks commented 9 years ago

oh right, and you need to distinguish it from the subsequent hundreds of test runs that passed before the test runner gave up. Got it.

firesofmay commented 9 years ago

@lackita Any updates on this one?

lackita commented 9 years ago

Sorry, I've been on vacation and haven't had much time to investigate. That's over tomorrow so I'll be looking at it some more on the commute home. On Aug 16, 2015 4:24 AM, "Mayank Jain" notifications@github.com wrote:

@lackita https://github.com/lackita Any updates on this one?

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-131503474 .

firesofmay commented 9 years ago

Sure, no problem :)

lackita commented 9 years ago

It looks like what happened is the spurious line was necessary when it was written, but then I later started capturing and propagating reports in another way and it became redundant.

firesofmay commented 9 years ago

@lackita Thanks will check it out. :) @gfredericks Any idea when are we releasing the next version?

gfredericks commented 9 years ago

Apparently this created a new issue (#23) so I will probably release when that gets figured out

gfredericks commented 9 years ago

Reopening this since it caused test failures.

firesofmay commented 9 years ago

@lackita Any updates on this?

lackita commented 9 years ago

Sorry, I'll try to take a look on the commute home today.

On Fri, Aug 28, 2015 at 7:51 AM, Mayank Jain notifications@github.com wrote:

@lackita https://github.com/lackita Any updates on this?

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-135748547 .

firesofmay commented 8 years ago

@lackita Hey any update on this?

lackita commented 8 years ago

@firesofmay Sorry, this got conflated with a conversations in #25, I believe the issue should be fixed now.

firesofmay commented 8 years ago

No problem. Thanks for all your help :) On Sep 3, 2015 2:06 AM, "lackita" notifications@github.com wrote:

@firesofmay https://github.com/firesofmay Sorry, this got conflated with a conversations in #25 https://github.com/gfredericks/test.chuck/pull/25, I believe the issue should be fixed now.

— Reply to this email directly or view it on GitHub https://github.com/gfredericks/test.chuck/issues/17#issuecomment-137237146 .

gfredericks commented 8 years ago

@firesofmay if you can confirm this fixes the problem I can make a release for it

firesofmay commented 8 years ago

@gfredericks LGTM! :+1:

gfredericks commented 8 years ago

Okay, this is released with 0.1.22