-
-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stepper breaks test-engine contract #214
Comments
I think that this line looks suspicious. Probably the When I make that change it seems to solve the original issue. @jbclements does that seem like the right fix to you? |
Hi there! Sitting in the airport in Amsterdam right now.
I just did 20 minutes of code spelunking, and it looks to me like I added this in 2008, in a commit that is now named f69ec2a … what I haven’t yet figured out is how it *ever* worked… it looks like the result of the code will always always be putting this in a list. I guess maybe that result isn’t used, but it’s both appalling to me that I didn’t find this sooner, and worrying to me that something else might now inadvertently depend on this. A comment (from 2008) suggests that the use-val-as-final tag is intended for use in test cases. I guess my next step is to take a look at where it’s used. Mike, I think you now know way more about the testing framework than I do, and you mention that this is not a new problem, it’s just resurfaced. Can you … tell me a bit more about that?
Long story short, I think Robby’s solution is almost certainly correct, so many thanks for finding this!
John
… On Dec 29, 2023, at 03:42, Robby Findler ***@***.***> wrote:
I think that this line <https://github.com/racket/htdp/blob/master/htdp-lib/stepper/private/annotate.rkt#L669> looks suspicious. Probably the (#%plain-app values results) should really be (#%plain-app apply values results).
When I make that change it seems to solve the original issue. @jbclements <https://github.com/jbclements> does that seem like the right fix to you?
—
Reply to this email directly, view it on GitHub <#214 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABXKOLC5M4W6MPKPKNV3C3YLYUZVAVCNFSM6AAAAABBCJPBWWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZRGY4DONBXHA>.
You are receiving this because you were mentioned.
|
@jbclements In the past, nothing used the return value of the tests, so that's why it was OK to ignore, and also why there was this lame |
Run this in the stepper with the current head of
racket/htdp
:You get
(This also shows up in the stepper tests.)
So somehow the return value from the check-expect test (which is supposed to be just plain
#true
) gets wrapped in a one-element list. I banged by head against this for a while, but am getting nowhere, unfortunately.This is not a new problem - it's just popped up again as the contract for
add-test!
was tightened.The text was updated successfully, but these errors were encountered: