racket / htdp

Other
91 stars 69 forks source link

Move coverage-increasing `void` out of `define-values`. #201

Open samth opened 12 months ago

samth commented 12 months ago

Fixes #200.

mfelleisen commented 12 months ago

@samth Do you know why the tests fail?

samth commented 8 months ago

The test failure is in tests/stepper/automatic-tests.rkt. @jbclements or maybe @mikesperber, do you know why this might be happening? Here is the output.

[samth@huor:~/.../extra-pkgs/htdp/htdp-test (master) plt] xvfb-run raco test  tests/stepper/automatic-tests.rkt 
raco test: "tests/stepper/automatic-tests.rkt"
raco test: @(test-responsible 'clements)
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((define-struct mamba (rhythm tempo)) (hilite (mamba-rhythm (make-mamba 24 2)))) ((define-struct mamba (rhythm tempo)) (hilite 24)))
test-sequence: test engine timeout while waiting for steps
...Error has occurred during test: 'define-struct
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (cons 1 empty)) (hilite (list 1 2 3))) ((cons 3 (cons 1 empty)) (hilite (cons 1 (cons 2 (cons 3 empty))))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (cons 1 empty)) (hilite (list 1 2 3))) ((cons 3 (cons 1 empty)) (hilite (cons 1 (cons 2 (cons 3 empty))))))
test-sequence: test engine timeout while waiting for steps
...Error has occurred during test: 'prims
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
test-sequence: steps do not match
   given: 'finished-stepping
expected: '(before-after ((cons 3 (hilite (cons 1 empty)))) ((cons 3 (hilite (list 1)))))
test-sequence: test engine timeout while waiting for steps
...Error has occurred during test: 'prims/non-beginner
mikesperber commented 8 months ago

@samth I can confirm this breaks the stepper. Here's a sample program:

#lang htdp/isl
(define-struct pare (kar kdr))

(define (add-pare pare)
  (+ (pare-kar pare)
     (pare-kdr pare)))

(add-pare (make-pare 23 42))

This completes immediately with the PR in place, with no output whatsoever.

mikesperber commented 8 months ago

@samth Maybe just go with @rfindler 's suggestion for now?

https://github.com/racket/htdp/issues/200#issuecomment-1726186306

samth commented 8 months ago

Does that fix the problem in #200? I would still like to actually do the right thing here, which I assume involves changing the stepper to cope with this change.

mikesperber commented 8 months ago

Does that fix the problem in #200?

Yes.

rfindler commented 8 months ago

I can't remember the details that I apparently unearthed a while ago, but I do remember enough to remain confident that the change I suggested in the comment that @mikesperber links to (starting with the comment "Ah. This is probably the right fix:") is a good change. Maybe other changes are also good, but that one seems to me to pretty clearly be right.

It seems likely to me that that code was trying to get something about coverage to work, but it was copying over only the source locations, where it needs to actually copy all the properties, not just the source locations.