Fix the stack trace of check-expect forms - #228
Conversation
When reporting "not given an inexact number" in check-within, the given
has to be formatted with ~s since it could have been a string:
(check-within (sqrt 2) 3/2 "0.1")
a3d8bd0 to
662cae4
Compare
| (define (do-check-expect test expected src) | ||
| (error-check (lambda (v) (if (number? v) (exact? v) #t)) | ||
| expected INEXACT-NUMBERS-FMT #t) | ||
| expected INEXACT-NUMBERS-FMT #t (list expected expected)) |
There was a problem hiding this comment.
I don't understand why expected is listed twice. This feels wrong.
There was a problem hiding this comment.
I changed the error message to:
check-expect: cannot compare inexact numbers, but the second argument is ~a. Try (check-within test ~a error-range).
Where the ", but the second argument is ~a" part is new.
So there are two ~a referring to expected now.
There was a problem hiding this comment.
Have you made sure that this change still works with the stepper?
There was a problem hiding this comment.
Yup! This PR does not change the expansion but only attaches an extra syntax property.
|
Thanks for getting this started. I don't have time to review the changes inside the file yet but three of them raise questions already. |
|
Can you do me a favor and check in Guillaume's paper whether "advice on how to fix things should go into error messages"? My recollection is that we should not do so unless obvious. I consider the suggestion in the modified inexact error message non-obvious. |
Sort of -- Guillaume Marceau (et al.) say
but also say that
which is not directly targeting this error message. The suggestion in the message has been there since 2010, do you want to remove it? |
|
Yes.
This is probably around the time Kathy was handing it over and Guillaume was running his research. If we pay attention to such results we should remove all these suggestions …
|
|
Tests added to |
|
@rfindler and I had a conversation about this issue at ICFP - Robby, could you look at this? |
|
Is this ready to be merged? |
|
I'm sorry, I've lost track of this conversation. I don't see any issues from a look at the diff, but if there is something specific I should be remembering, could you remind me @mikesperber ? |
(#689) * Add some tests for racket/htdp#228, racket/htdp#229, and racket/htdp#230 to `tests/drracket/module-lang-test`. + Tests of racket/htdp#229 should first save the buffer to disk before running + Some racket/htdp#229 tests are ported from htdp-test:intm-lam.rktl * Update `tests/drracket/language-test` to match racket/htdp#229. * Manually check for empty stderr in tests/drracket/module-lang-test Somehow `-e`/`--check-stderr` does not work with `tests/drracket/module-lang-test` (perhaps due to using `test-log` + `exit 0` in `fire-up-drracket-and-run-tests`?) * Test utils: sleep for 0.1s after printing error msg In tests/drracket/module-lang-test, stderr goes through a pipe to be checked for the absence of error messages. Therefore, sleep for 0.1s before existing to let the background thread pipe the messages to terminal.
Backported from racket/drracket@0375cc82ab4e08.
4 participants
In #lang SLs,
check-expectand similar forms are not annotated by errortrace.This PR adds the
errortrace:annotatesyntax property to request for annotation.Before:
After: error at line 3, column 0 and line 5, column 0.
In addition to stack trace fixes, this PR refines and fixes somes
check-form error messages:check-randomandcheck-expectshould NOT have identical error messages.check-expectmessages refer to "the second argument" like othercheck-forms.check-withinwith~sinstead of~afor otherwise strings like"NNN.mm"would be printed like numbers (i.e. justNNN.mm).