Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

samth
Copy link
Sponsor Member

@samth samth commented Sep 18, 2023

Fixes #200.

@samth samth mentioned this pull request Sep 18, 2023
Open
@mfelleisen
Copy link
Contributor

@samth Do you know why the tests fail?

@samth
Move coverage-increasing void out of define-values.
d0dcd07 
Also copy properties appropriately to support `'errortrace-annotate`.

Fixes racket#200.
@samth
Copy link
Sponsor Member Author

samth commented Jan 7, 2024

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
Copy link
Member

@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
Copy link
Member

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

#200 (comment)

@samth
Copy link
Sponsor Member Author

samth commented Jan 14, 2024

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
Copy link
Member

Does that fix the problem in #200?

Yes.

@rfindler
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test coverage false negative in #lang htdp/bsl
4 participants
@samth @mfelleisen @mikesperber @rfindler