search

jorgenschaefer / emacs-buttercup

Behavior-Driven Emacs Lisp Testing
GNU General Public License v3.0
360 stars 44 forks source link

Inexplicable hang on Emacs 30 snapshot #247

Open alphapapa opened 4 days ago

alphapapa commented 4 days ago

Hi,

I'm seeing that my tests for org-ql with Buttercup are hanging exclusively on Emacs snapshot (currently an Emacs 30 pre-release version). I don't know if this indicates a bug in Buttercup, or an incompatible change in Emacs 30, but I hope that we can find the problem and solve it before Emacs 30.1 is released. These hangs are happening with the latest version of Buttercup installed automatically from MELPA during the CI run.

Perhaps notably, perhaps related, Guix has disabled tests for org-ql due to what seems to be a similar problem: https://git.savannah.gnu.org/cgit/guix.git/commit/gnu/packages/emacs-xyz.scm?id=3add97c7761e6c58a1d7405f417a49dda5f0a742

* gnu/packages/emacs-xyz.scm (emacs-org-ql): Update to 0.8.6.
[arguments]<#:tests?>: Disable tests, which freeze starting with recent
Buttercup releases.

I can't reproduce the problem locally, being on a different Emacs version. But besides that, earlier today while debugging similar issues, I had similar hangs during a Buttercup run on other Emacs/Org/Buttercup versions, and I don't know how to "look inside" a Buttercup run to find out what's hanging.

Any suggestions would be appreciated. Thanks.

snogge commented 3 days ago

I was able to reproduce. It not only hangs, it also eats a lot of memory. This is probably the stacktrace generation and the fact that buttercup does not use print-circle t. The reason for the failing tests is a change in read in Emacs 30. This is probably unintended as I cannot see any mention of this changed behavior in NEWS. It also looks like it should still work. I have not filed any Emacs bug, and I have not searched the bug database.

alphapapa commented 3 days ago

Wow, that must have taken a while to research. Thanks for digging in to that. Have you found the commit in emacs.git that makes the incompatible change to read?

alphapapa commented 3 days ago

After looking at your comment on https://github.com/alphapapa/org-ql/pull/442, I remembered something I saw recently: I wonder if the problem could be this: https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f2bccae22bd47a2e7e0937b78ea06131711b935a (NEWS entry)

snogge commented 2 days ago

I just tried to isolate the hanging test, and once I got the test suite to actually exit I noticed some failing tests.
Interestingly, disabling all but the hanging test also resolved the infloop. I'm guessing this means you have some dependency between your tests. Anyway, once I found the first failing test it was fairly easy to find the naive solution of just taking the car before calling read. But there was probably an element of luck involved.

The Emacs change you point out could be responsible for this breakage. I don't understand the C parts of Emacs very well. The patch does not touch the obvious pieces of lread.c but there are probably interactions that I do not see.

Looking at my comments on https://github.com/alphapapa/org-ql/pull/442 I probably drew the wrong conclusion as to why the car was needed. Still not sure I understand it.

alphapapa commented 2 days ago

@snogge It seems that, once the accidental misuse of read is fixed, (e.g. see https://github.com/alphapapa/org-ql/tree/wip/test-fix-read-issue where I made the changes in a test branch--I will credit you when merging for "real"), all of the tests pass. (Apparently I never noticed that I was calling read with a list instead of a string; I don't know why it ever worked. The comment there mentions that url returns an "improper alist", but I must never have noticed that it needed a further step to access the value properly.)

On the Buttercup side, are there any improvements that could be made to prevent its apparently hanging in a situation like this? Or that would make it easier for an end-user like me to debug?

Thank you for digging into this problem. I regret that it turned out to be caused by a minor bug in org-ql, but hopefully this discovery will benefit Buttercup as well.