racket / ChezScheme

Chez Scheme
Apache License 2.0
110 stars 8 forks source link

update github test runners #59

Closed burgerrg closed 1 year ago

burgerrg commented 1 year ago
mflatt commented 1 year ago

Maybe the right repair for the flaky test is to remove (make-time 'time-duration 0 60) from line 159? That way, the test doesn't depend so much on finishing within a specific time.

I'm guessing that the timeout was there to avoid getting stuck if something goes wrong. We could also make the timeout 10 times as long, but it seems better to avoid a timeout if it isn't needed.

mflatt commented 1 year ago

I tried removing (make-time 'time-duration 0 60), and in one case, the same platform got stuck and timed out at this test. I'm investigating more.

mflatt commented 1 year ago

After staring at the test long enough, I finally see that there's race between threads finishing and the main thread taking the mutex m to wait on the condition c. If all the threads finish before the main thread gets the mutex, it never sees a signal. That might be why the test sometimes fails. I'm trying the repair of moving with (with-mutex m ...) to surround the thread creation.

burgerrg commented 1 year ago

That was a good find! I'm trying the update now in this PR.

burgerrg commented 1 year ago

My fix is similar to yours. I removed the unnecessary definition of iota and kept the 60 second timeout, which should be at least ten times what's needed. I like having it return the result when it doesn't match so that we can have some idea of what went wrong.

mflatt commented 1 year ago

Thanks!