How to update check-info location?

[bennn]

Here's a macro that uses with-check-info* to set a check's location relative to its argument.

#lang racket/base
(require rackunit (for-syntax racket/base syntax/parse syntax/srcloc))

(define-syntax (check-true* stx)
  (syntax-parse stx
   [(_ . e*)
    #`(begin .
        #,(for/list ([e (in-list (syntax-e #'e*))])
            #`(with-check-info* (list (make-check-location '#,(build-source-location-list e)))
                (λ () (check-true #,e)))))]))

(check-true*
 #true
 #false)

Running this on HEAD prints an error with 2 location:s

--------------------
FAILURE
location:   test.rkt:14:1
name:       check-true
location:   test.rkt:10:22
params:     '(#f)
--------------------

Running on 6.5 prints only one:

--------------------
FAILURE
name:       check-true
expression: (check-true #f)
params:     (#f)
location:   (#<path:/Users/ben/code/racket/gtp/shallow/model/test.rkt> 14 1 387 6)

Check failure
--------------------

Should I change my macro?

[jackfirth]

I think your macro is correct, and the current behavior is buggy. The question comes down to whether the following two expressions should be equivalent:

(with-check-info (['foo 1])
  (with-check-info (['foo 2])
    (fail-check)))

(with-check-info (['foo 2])
  (fail-check))

The continuation mark implementation declared (but didn't test) that yes, these are equivalent. As a RackUnit user this makes sense to me, and it allows custom checks to "override" the default check infos as yours has done. This doesn't allow custom checks to remove the default infos (although they can make them verbose), which might be a problem theoretically but I'm not too bothered by it.

Should updating an info preserve its original position in the info stack or append it to the end of the stack, updating its position? Original position makes the most sense to me, and mimics the old behavior that was hacked in by sorting the check info stack (it only preserved the old position in the stack for the "special" infos added by define-check, and only by forcing an order on them).

[jackfirth]

I've realized that the old behavior makes it impossible to deliberately include multiple infos with the same name, which I needed for racket-expect. There's two ways to proceed:

• Declare that the new behavior is correct, and add some sort of (update-check-info* infos thnk) procedure for replacing existing check infos instead of adding new ones. This is simple to use and implement IMO, and avoids surprising behavior. But it means RackUnit will soft-break backwards compatibility by messing with the info stacks of custom checks that rely on the overriding behavior like yours does.
• Declare that the old behavior is correct, and add some sort of (repeated-info info-values) struct that's like nested-info but makes the same check-info name print multiple times with different values instead of printing an indented subsequence of infos. This suits racket-expect just fine and preserves backwards compatibility, but it wouldn't support adding repeated check infos that aren't grouped together in the info stack like (with-check-info (['foo 1] ['bar 2] ['foo 2]) body ...) does. It also makes the info stack data structure a little more confusing to reason about: it's no longer just a list that with-check-info appends to, it's this weird hash-with-order-and-possibly-multiple-values-per-key thing. It wouldn't be much more complicated, but it wouldn't be simple either.

Personally I lean towards option one, but breaking checks like yours worries me.

[bennn]

Option 1 is okay with me, as long as we document that the "check info stack" is a list.
(This data structure's currently undocumented, right?)

Why do you need multiple infos with the same name in racket-expect?

[jackfirth]

@bennn Do you think this should be resolved before the release goes out? If so I'll open a PR and get it merged before #59 so the version history has update-check-info* added in 1.8 and the meta checks added in 1.9. That will make cherry picking the PR into the release easier.

The check info stack is documented as a list of check info values in the stack field of the exn:test:check exception, but other than that the docs make no note of it. In particular, the de-duplication / overriding behavior of using the same info name multiple times is undocumented.

For racket-expect an expectation could find multiple "faults", and each one is added as a check info value. They're all named "fault". Each one is a nested-info, so having a single info with a list wouldn't properly display the nested infos.

[bennn]

Yes I think it'd be best to have update-check-info* in the release

[bennn]

#64 didn't fix this. If I run the original program:

#lang racket/base
(require rackunit (for-syntax racket/base syntax/parse syntax/srcloc))

(define-syntax (check-true* stx)
  (syntax-parse stx
   [(_ . e*)
    #`(begin .
        #,(for/list ([e (in-list (syntax-e #'e*))])
            #`(with-check-info* (list (make-check-location '#,(build-source-location-list e)))
                (λ () (check-true #,e)))))]))

(check-true*
 #true
 #false)

The error message references line 10 (inside the macro)

--------------------
FAILURE
name:       check-true
location:   test.rkt:10:51
params:     '(#f)
--------------------

because check-true puts its info on the stack after the with-check-info* does.

---

Maybe the checks need to add a location only if it's not already on the stack?
Is that what it did before?