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.

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

Introduce a new envvar to set a longer timeout for Debian buildds #737

Closed

Conversation

manphiz
Copy link

@manphiz manphiz commented Jan 18, 2025

  • Some of the Debian supported architectures are very slow and requires a longer timeout for some of the tests to finish.

* Some of the Debian supported architectures are very slow and
requires a longer timeout for some of the tests to finish.
@greghendershott
Copy link
Owner

Thanks for the pull request!

At first I didn't understand this. But digging around, I'm guessing this is this for running the Emacs Lisp tests on a buildd instance, for the Debian package called racket-mode?

Can you help me understand the purpose of that Debian package?

Why I ask: Racket Mode is a combination of both (a) Emacs Lisp and (b) Racket code. It is designed for both the Emacs Lisp and Racket code to be updated in sync and delivered together as an Emacs package (or people can install both directly from source, here).

It is definitely not designed/maintained to support people getting some of the code via one method, and some via another method. [In other words, I deliberately don't supply both an Emacs Lisp package and a separate Racket package, and ask users to update both. In fact, I frequently change the API between the two, in ways that don't matter when they're delivered together -- but could break badly if delivered separately and not updated perfectly in lock step.]

At a quick glance at the Debian package's test logs, I see only the Emacs tests being run (e.g. make test-elisp). But I don't see the Racket tests being run (make test-racket). The right thing to do is run both, (make test).

So I'm a little concerned that the Debian package might not be doing the right thing?

Can you help me understand more? Thanks!

@manphiz
Copy link
Author

manphiz commented Jan 19, 2025 via email

@manphiz
Copy link
Author

manphiz commented Jan 19, 2025 via email

@greghendershott
Copy link
Owner

Thank you very much for all the context/information in your previous reply!

Xiyue Deng @.***> writes:
Now for the PR, the purpose is to make the ELisp tests pass reliably on Debian buildds of slower architectures. Currently the only architecture that can finish with a timeout of 60 seconds on any operation is amd64. And when running manually, it looks like starting a REPL session takes a long time, and requires a timeout of 5 minutes on i386, and as long as 15 minutes timeout on arm64. Maybe racket is not well optimized on other architectures yet. Is the approach done in the PR acceptable? And let me know if this can be handled in a better way.
Actually, it probably helps to disable the timeout altogether so that on slower architectures we just wait for the tests to finish. Is this possible?

-- Regards, Xiyue Deng

The timeout is c. 10 years old. I don't recall exact examples. In general:

There are some strict "unit" tests that, if they will ever pass/fail, will do so in seconds or minutes -- but some "integration" tests might never complete at all if there's a bug/problem.

[Not only does this include an infinite loop in synchronous code. There's also commands from the Emacs front end to the Racket back end -- these are non-blocking; the back end sends a completion message asynchronously, later. So there exist some tests that must loop checking for a command response, or the desired effect from one, repeatedly until the timeout. :( ]

So a timeout has been useful when running the tests locally (e.g. on my or a contributor's personal machine).

It also has helped on remote CI (originally Travis, lately GitHub Actions), because sometimes a human is waiting on those results (e.g. before reviewing or merging something). Eventually (~ an hour) GHA will terminate the runner, but that's a long time for a busy human to wait for a test that should have passed/failed in seconds or minutes.

For buildd, if no human is waiting starting at the screen, and if the buildd instances eventually get killed and cleaned up (do they?), then I agree the tests themselves don't need a timeout.

Because the timeout concept is woven through the tests, it's probably simplest for you to use your original PR, and just change the 900 literal to a Very Large Number. Probably the Emacs Lisp constant most-positive-fixum?

@manphiz
Copy link
Author

manphiz commented Jan 20, 2025 via email

greghendershott added a commit that referenced this pull request Jan 20, 2025
Also incorporate the substance of the changes from PR #737.

Remove `racket-tests/eventually`, which was intended as a helper
function and is a footgun when used directly in tests.

Change how racket-tests/racket-repl detects the REPL buffer.

Update the looking-back tests because the REPL now has a prompt like
"repl.rkt>" instead of just ">".

Revive the commented-out multiple expression tests, because I revived
that functionality quite awhile ago.
@manphiz manphiz deleted the longer-timeout-for-debian-buildd branch January 21, 2025 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants