Add meta checks

[jackfirth]

Closes #47

This PR adds the following checks:

• (check-fail <pred-or-exn> <thunk>) - asserts that <thunk> evaluates a failing check and that the check failure exception satisfies <pred-or-exn> in the same way as check-exn
• (check-fail* <thunk>) - like check-fail, but without caring about the specifics of the thrown failure
• (check-fail/info <check-info> <thunk>) - asserts that <thunk> evaluates a failing check and that the check failure exception contains an info that is equal? to <check-info>
• (check-error <pred-or-exn> <thunk>) - asserts that <thunk> evaluates a check that raises an error instead of failing and that the raised value satisfies <pred-or-exn>

I went with check-fail/info instead of adding an info case to check-fail to avoid confusion over whether check-error has an info-accepting variant. The existence of check-fail/info and nonexistence of check-error/info is a clear signal in documentation search results and playing at the REPL that wouldn't be present if check-fail/info was merged with check-fail.

The only addition from the proposed API is check-error, which I realized I'd need to test to the contract failure cases of these checks.

[AlexKnauth]

Why is check-error called check-error?

[jackfirth]

Symmetry with the output of checks, since check failures print "FAILURE" while check errors print "ERROR". If you have a suggestion for a less ambiguous name, I'm all ears.

[bennn]

How does check-error differ from check-exn?

I tried a quick example and check-exn works how I'd expect:

#lang racket
(module+ test
  (require rackunit)
  (check-exn exn:fail:contract?
    (lambda () (check-true (+ "asdf" 2)))))

$ raco test test.rkt
raco test: (submod "test.rkt" test)
1 test passed

Reminds me though, it would be good to have a check-exn helper to test "throws a contract error that blames a certain party"

[jackfirth]

That example works because (+ "asdf" 2") is evaluated before being passed as an argument to check-true, so the error isn't thrown from within the body of check-true. Try this example:

(check-exn (lambda (v) (equal? v 'oops))
  (lambda () (check-pred (lambda (v) (raise 'oops)) 'foo)))

[jackfirth]

@bennn Added docs and addressed all your comments (I think), this PR is ready for a final review.

[AlexKnauth]

Would it be better if check-error were renamed to either check-check-error or check-fail-error?

[jackfirth]

@AlexKnauth How about check-fail/error? According to scheme pronunciation rules, that's "check fail with error" which seems accurate.

[jackfirth]

@bennn For some reason I can't respond directly to your comments about procedure-arity-includes?. I added the arity check in contract-pred-or-msg! so I don't think it needs to be tested in the cond expression.

[AlexKnauth]

(Re: check-fail/error) That makes sense to me.

[rmculpepper]

I think this should be a new module, maybe rackunit/meta. That would make it clearer that these are not general-purpose checks, but rather specialized for testing other checks.

I'd prefer if check-fail, check-fail*, and check-fail/info were combined into one form. (I disagree with your rationale for keeping them separate.) I propose:

;; check-fail : (Treeof FailurePred) (-> Any) -> Void
;; where FailurePred is one of
;; - (exn:test:check -> Boolean)  -- exn must satisfy predicate
;; - Regexp                       -- exn message must match
;; - Check-Info                    -- exn must contain check-info

More descriptive, less ambiguous names would be check-check-fail and check-check-error... but I could understand not liking them. check-fail/exn is not quite right, since we consider failures distinct from errors. But check-error is bad, because it's too similar to check-exn. (I believe there's a library somewhere that defines check-error as a macro that wraps the expression to check as a thunk.)

[jackfirth]

@rmculpepper Thanks for taking a look at this! Here are my thoughts:

A rackunit/meta module is a good idea, but why stop there? Should there be rackunit/base, rackunit/check, rackunit/suite, or other modules for splitting up the contents of rackunit? I'd find it strange to only have rackunit/meta provided from a special module while nothing else in RackUnit's non-UI code is modularized. What about if rackunit provides the meta checks and we open a separate issue for finer-grained modules?

To me, the (Treeof FailurePred) argument API is too complex. It presents multiple ways of doing the same thing, e.g. these are all nearly equivalent:

(test-begin
  (check-fail (list #rx"foo" pred) thnk))

(test-begin
  (check-fail #rx"foo" thnk)
  (check-fail pred thnk))

(test-begin
  (check-fail (list (list #rx"foo") (list (list pred))) thnk))

One of these could be preferred, but will they have subtle differences that a user has to remember? What about the extra location info provided by separate checks? And when using only a single predicate / regex / info, does it needs to be wrapped in a list to be considered a tree? Can you mix predicates / regexes / infos in the same tree? Is the order important? Is the tree traversed depth-first or breadth-first? Why is it a tree and not a list? If multiple parts of the tree fail, which message is shown? These are all questions I'd have to ask myself as a user of this API, and I'd likely forget the answers to some of them frequently and have to consult the docs. Not to mention there's the increased implementation and contract complexity. I don't see the benefits of this approach outweighing the costs.

[bennn]

I like @rmculpepper 's suggestions.

• :+1: for rackunit/meta, because it distinguishes "RackUnit for the meta-tester" from "RackUnit for the normal user".
  • it's fine with me to add rackunit/base etc. as long as (require rackunit) still gives the same imports.
• :+1: for having one form
• :+1: for accepting a tree. The plot library has a similar (very convenient) API for renderers. I think it should check everything in the tree & report all errors. Order should be unspecified --- if you want to meta-meta-test then only supply non-list arguments

[jackfirth]

Re rackunit/meta: I will move these exports to rackunit/meta and not export them from rackunit. This PR will not attempt to add a rackunit/base module or make any changes to the current exports of rackunit.

As for the tree API:

A tree API wouldn't quite be a single form, because it would still need separate forms for normal failures and errors. A third form that that supplied an empty tree by default would also be needed if we wanted to match the simplicity of check-fail*.

A tree of renderers in plot makes sense because there would be a lot of duplication otherwise, and there's no semantic changes that can occur as a result of the order of renderers in the tree. Changing the order of values in the tree supplied to a tree-based meta check form would change which error is shown, and I don't see any circumstance where the message argument to such a check would apply to all leaves of a tree.

A tree API also makes it harder to distinguish between different kinds of failures. Each leaf of the tree would have the same check location information. DrRacket's integration with the location information makes it easy to instantly see exactly which cases failed; I consider it bad practice for a rackunit API to encourage users to subvert that feature. Maybe if we had some way to point to different leaves of the tree in the location message?

[bennn]

I still like the tree API

• I don't mind losing check-fail*
• RackUnit can test everything in the tree (so changing order only changes the orders of errors, not "which errors are shown")
• to distinguish kinds of errors, you can split one check-fail* into multiple (check-exn has the same "problem" --- a user can write one big predicate that tests lots of things & is hard to debug)

[jackfirth]

I'd like to get this feature in and it seems I'm the only one doubting a tree API, so I shall defer to your judgment. Tree API it is!

With multiple errors, how should the checks report failures? Multiple failures for one check is something I've been working on in racket-expect and it can be complex enough that I'd rather avoid implementing it entirely here.

[bennn]

I think just normal messages separated by whitespace would be OK. Example:

--------------------
FAILURE
name:       check-equal?
location:   foo.rkt:4:0
actual:     1
expected:   0

name:       check-equal?
location:   foo.rkt:5:0
actual:     1
expected:   2
--------------------

But since name & location are always the same, maybe each failed check in the tree could register a unique check-info key & all these keys could be printed on failure.

[jackfirth]

Just printing like this would be fine with me:

--------------------
FAILURE
name:       check-fail?
location:   foo.rkt:4:0
actual:     1
expected:   0
actual:     1
expected:   2
--------------------

Anything more complicated I'd defer to a separate package designed with multiple failures in mind.

[bennn]

Can simplify 1 more step and only print actual 1 time? So check-fail prints one actual & multiple expecteds

[jackfirth]

This is ready for another review.

Now, the only export of rackunit/meta is check-fail. It accepts a tree of predicates, regexps, and check info values. Here is a sample use and its output:

(define (fail-foo)
  (with-check-info (['foo 1])
    (fail-check "some message from foo")))

(define (has-bar-info? exn)
  (member 'bar (map check-info-name (exn:test:check-stack exn))))

(check-fail (list has-bar-info?
                  #rx"message from bar"
                  (make-check-info 'bar 1))
            fail-foo)

--------------------
FAILURE
name:            check-fail
location:        unsaved-editor:13:0
actual:          (exn:test:check "some message from foo" #<continuation-mark-set> ...)
actual-message:  "some message from foo"
actual-info:
  foo:             1
expected:
  predicate:       #<procedure:has-bar-info?>
  message:         #rx"message from bar"
  info:            (check-info 'bar 1)
--------------------

The check-fail/error form is no longer needed because #75 means check failures evaluated in thunks given to check-exn will raise their exceptions normally and not log failures (hooray!)

See the meta-test.rkt module for a more substantial example of how one would test custom rackunit checks.

[jackfirth]

Sorting and grouping infos in the failure message is now implemented.