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

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

Closed
alphapapa opened this issue Dec 10, 2022 · 6 comments
Closed

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

alphapapa opened this issue Dec 10, 2022 · 6 comments

Comments

@alphapapa
Copy link
Contributor

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
Copy link
Collaborator

snogge commented Dec 13, 2022

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
Copy link
Collaborator

snogge commented Dec 22, 2022

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

@snogge
Copy link
Collaborator

snogge commented Dec 22, 2022

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

@snogge
Copy link
Collaborator

snogge commented Jan 22, 2023

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 alphapapa/org-ql#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
Copy link
Contributor Author

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!

alphapapa added a commit to alphapapa/emacs-package-dev-handbook that referenced this issue Mar 9, 2023
See <jorgenschaefer/emacs-buttercup#228>.

(Somehow org-export is broken due to a bug in prism, so the HTML still
needs updating.)
@snogge
Copy link
Collaborator

snogge commented Mar 9, 2023

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

@snogge snogge closed this as completed Mar 9, 2023
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

No branches or pull requests

2 participants