jorgenschaefer / emacs-buttercup

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

Bizarre failure to match error signal message with :to-throw #228

Closed alphapapa closed 1 year ago

alphapapa commented 1 year ago

Hi,

I'm stumped. Please see this CI run: https://github.com/alphapapa/org-ql/actions/runs/3662214898/jobs/6191147365#step:6:1008

You can see there that what Buttercup says it is expecting appears to be what is thrown:

FAILED: Expected `"\303 \210\301\304\300\242\302\305\300\242%\207"' to throw a child signal of `user-error' with args `("Views that search non-file-backed buffers can't be linked to")', but instead it threw (user-error "Views that search non-file-backed buffers can’t be linked to")

And, in fact, when I run this test on my local system, it passes, using Emacs 28.1. But when run on GitHub CI, it fails, as you can see.

Bizarrely, if one copies the two strings from the GitHub CI Web interface into Emacs and tests them with equal, the strings do not match. Closer inspection reveals that one of them has an ASCII apostrophe while the other has a Unicode RIGHT SINGLE QUOTATION MARK.

So one might think that, somehow, the same had happened in the source code (which wouldn't explain why the test passes locally, but anyway). Yet this is not the case: in the source file, an ASCII apostrophe is used: https://github.com/alphapapa/org-ql/blob/d253b123cf5bf869857ef978e40e2d42c2d301f4/org-ql-view.el#L675 And in the test file, one is also used: https://github.com/alphapapa/org-ql/blob/d253b123cf5bf869857ef978e40e2d42c2d301f4/tests/test-org-ql.el#L2203

So I've no explanation for why this test fails. My best guess is something related to a locale difference on the CI container, but that still doesn't make sense, because in the source code, the strings are the same.

Thanks for your work on Buttercup.

snogge commented 1 year ago

This was a tricky one. The problem is that user-error messages (possibly other errors?) are processed with substitute-quotes. So the strings mismatch on string element 45, apostrophe ' in the expected string, right single quotation mark (0x2019) in the received string.

I'm not sure if buttercup should do anything with the expected string. It might be better to leave that to the tests.

The mismatch explanation should be improved, I debugged this by using the equal ert-explainer on the signal arguments. Once that was in place the problem was obvious. But currently all variants of faliure messages are always generated, and that is too expensive and makes the tests take way to long to run. So the matchers needs to be rewritten, at least the :to-throw one.

snogge commented 1 year ago

Hi @alphapapa , did this help you figure out your failing tests?

snogge commented 1 year ago

And re-reading your original report, I see you already figured out that it was the apostrophe that was the problem.

snogge commented 1 year ago

The decision process for quote substitution in substitute-quotes depends on the value returned by the C function text-quoting-style, which in turn depends on the variable text-quoting-style and some checks for system/terminal supports. When the variable text-quoting-style is nil - the default - Emacs will call default_to_grave_quoting_style to see if system supports curved quotes. If not, substitute-quotes will use the style grave which will leave apostrophes/ASCII single quotes alone.

The tests in default_to_grave_quoting_style are

  1. Does the current system have locale use UTF-8? If not, use style grave.
  2. Is standard-display-table non-nil and a display table? If not - the default is nil - use style curved
  3. Check if the standard-display-table entry for LEFT SINGLE QUOTATION MARK is actually the GRAVE ACCENT? If so, use grave otherwiswe curved.

At least that is how I understand the C code. In this I case I would guess that you get style grave locally because you do not use an UTF-8 locale while running tests locally. The UTF-8 test also check that the system have wchar_t and is not WINDOWSNT, so I guess it could be either of those two as well. But I find that unlikely.

I see suggested in https://github.com/alphapapa/org-ql/issues/317 that you could use (user-error "%s" "...don't...") to avoid this problem. Another way is to use

(expect (let ((text-quoting-style 'grave))
      (user-error "`bar'"))
    :to-throw 'user-error '("`bar'")))
alphapapa commented 1 year ago

Hi Ola,

Sorry for the delay in getting back to this. Haven't had time to dig into org-ql stuff for a while.

FWIW, here's what locale outputs on my GNU/Linux system:

LANG=en_US.UTF-8
LANGUAGE=
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=

Thanks for the suggestion to use (user-error "%s" "...don't...") to work around the problem. With that I'll finally be able to have the test suite passing again.

Anyway, I suppose this problem isn't Buttercup's fault, but it would be nice to have it documented somewhere. Maybe it's time for Buttercup to have a FAQ or troubleshooting section in the docs? :) In the meantime I guess I'll have to document it in my package dev handbook...

Thanks for digging into this!

snogge commented 1 year ago

The credit for the (user-error (format "...")) trick should go to @stefanv in https://github.com/alphapapa/org-ql/pull/323.
I'll close this issue now.