Closed bennn closed 6 years ago
The PR & issues about check-exn
ignoring its message argument are/were related to this (#65).
I'm thinking, as a general rule, the checks
in RackUnit should not add things to the check info stack if there's matching info already on the stack. (Like default arguments to a function.)
Many of my tests failed to report the right source location, due to a change that occurred between 6.10 and 6.11.
This pr fixes my issue.
I think this is the right thing to do and think you've got the right rule here:
I'm thinking, as a general rule, the checks in RackUnit should not add things to the check info stack if there's matching info already on the stack. (Like default arguments to a function.)
I likely won't have time to review this (or do anything else with RackUnit) this week or weekend but I'm planning on getting to it by the end of the month. If before then you'd like to make a PR that makes all "automatic" check infos added by define-check
behave like this, I have nothing but encouragement.
I changed the docs (and code) to say that define-check
adds 'location
and 'expression
if they aren't already there.
~This is different from the rule I proposed above, but I think it makes sense based on todays reading of the documentation. I'll re-read next weekend.~ EDIT: that was not a good idea; in practice I think people want to be able to define new checks from existing ones, and to do that well you want to be able to override everything that a basic check will do by default
Updated,
with-default-check-info*
--- like with-check-info*
but does not override existing infos
Change
define-check
to only add a location for the check if thecurrent-check-info
stack does not have a location on it.(I think this is what
check-true
and others used to do. I'm not sure if they did the same for other check-info items too.)With this PR,
this program fails with a good error message:
Without this PR,
the above program needs to be rewritten something like this to get a good error message:
In case we'd rather close this PR without merging, I looked in the main distribution for uses of
with-check-info
that updated the location. The only ones I found were in therackunit-abbrevs
andtyped-racket-test
collections. I can change those to do thecall/updated-location
dance.