htdp icon indicating copy to clipboard operation
htdp copied to clipboard

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

Open samth opened this issue 1 year ago • 7 comments

Fixes #200.

samth avatar Sep 18 '23 21:09 samth

@samth Do you know why the tests fail?

mfelleisen avatar Sep 18 '23 21:09 mfelleisen

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

samth avatar Jan 07 '24 16:01 samth

@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 avatar Jan 14 '24 17:01 mikesperber

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

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

mikesperber avatar Jan 14 '24 17:01 mikesperber

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.

samth avatar Jan 14 '24 17:01 samth

Does that fix the problem in #200?

Yes.

mikesperber avatar Jan 14 '24 19:01 mikesperber

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.

rfindler avatar Jan 14 '24 20:01 rfindler