shriram / gradescope-racket

Infrastructure to autograde Racket code on Gradescope
MIT License
17 stars 8 forks source link

Discussion: What's the right way to add timeouts for testing? #22

Closed jasonhemann closed 2 years ago

jasonhemann commented 2 years ago

My students' will sometimes accidentally code infinite loops. As written, the entire test suite will time out when I test such code, which isn't as helpful as it could be.

  1. What's the right way to flexibly provide good testing of students' code that may infinite loop? Here's what I'm currently using:
(require racket/engine)

(define timeout-symbol (gensym 'timeout))

(define-syntax-rule (run-w/timeout e)
  (let ([w/timeout (engine (λ (f) e))])
    (if (engine-run *TIMEOUT-MS* w/timeout)
        (engine-result w/timeout)
        '(timeout-symbol "test timed out"))))

I'm currently needing to wrap this around almost all my tests, which seems gross and wrong. In principle I'll want to timeout on almost any tests, b/c they could accidentally loop in any program they write. It's a shame to have to set one single time-out.

@wilbowma , are you currently using https://gist.github.com/wilbowma/79330280f474ecc456916787028206cc ? Matthias suggests: https://www.mail-archive.com/racket-users@googlegroups.com/msg32199.html on So what's the right API and a good standard implementation to support it?

  1. This seems like something that should be a part of or an extension to rackunit, rather than hand-rolled each time. I assume there's an obvious reason rackunit doesn't include some solution for this already: what is that reason?
wilbowma commented 2 years ago

I currently use something closer to Matthias' suggestion (https://github.com/cpsc411/cpsc411-pub/blob/3033dfd6db0d25122d94cfd924e065e76a44231a/cpsc411-lib/cpsc411/test-suite/utils.rkt#L294), because I ported my test suite to generate almost everything from a little DSL for compiler tests. Before that, it was too difficult to insert manual termination checks for every input, so I needed something like my gist.

I prefer the explicit termination check as it gives you better control over error messages and fewer false positives if you have long running tests.

rymaju commented 2 years ago

https://github.com/rymaju/gradescope-racket/commit/53c2eaf0638b9f1acaf0f29ccd6f1e33947f2041

This seems to work? Not sure if a more experienced Racketeer could chime in on whether this is robust enough. Of course this should be optional as well -- maybe through passing in some optional arguments / a parameter?

rymaju commented 2 years ago

Unfortunately, it seems like the best way to do this would be to time out each individual test with something like racket/engine, I would have loved to run everything async, but that only works if you don't have any before or after setup/teardown.

shriram commented 2 years ago

Fwiw, @rymaju, I definitely like your proposal!

jasonhemann commented 2 years ago

closing via #24