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

Tests are broken in release 3.6.2 #10134

Closed
zanchey opened this issue Dec 4, 2023 · 7 comments
Closed

Tests are broken in release 3.6.2 #10134

zanchey opened this issue Dec 4, 2023 · 7 comments

Comments

@zanchey
Copy link
Member

zanchey commented Dec 4, 2023

I thought I'd run the test suite, but it looks like I didn't.

Testing file checks/basic.fish ... Failure:

  The CHECK on line 596 wants:
    \\Xef\\Xb7\\X92foo

  which failed to match line stdout:98:
    \\$\\\\ufdd2foo

  Context:
    [...] from line 542 (stdout:95):
    bar
    baz
    C
    \\$\\\\ufdd2foo <= does not match CHECK on line 596: \\Xef\\Xb7\\X92foo
    ./argv0.sh

The check is looking for the UTF-8 encoding.

I've already pushed the advisory and the new release, so I can make a 3.6.3 to fix the tests.

@sbradnick
Copy link

I get the reverse, 3.6.2 did NOT have this issue, but 3.6.3 DOES print this "Failure".

@cole-h
Copy link

cole-h commented Dec 4, 2023

I'm seeing the same -- 3.6.3 has this failure but 3.6.2 does not.

@zanchey
Copy link
Member Author

zanchey commented Dec 4, 2023

Thanks. @faho reckons they pass on his machine as well, but they fail on my local machines and on all CI platforms with 3.6.2. So more digging to be done, but it's way past my bedtime here!

faho added a commit to faho/fish-shell that referenced this issue Dec 4, 2023
This test wants to generate a U+FDD2 to see it is not mishandled.

To do so, we tried to use sh, which on my system is bash and can do
`$'\ufdd2'`.

Unfortunately on other systems it might be dash, which won't do that.

Since I don't know of a good no-dependency portable way to generate
this (I dimly remember python3 being a shim on some systems, so I do
not want to invoke it here), we'll just use our own printf.

Which is a worse test, we control both parts, but it'll do.

Fixes fish-shell#10134
@faho
Copy link
Member

faho commented Dec 4, 2023

The issue is /bin/sh - on my system that is bash, on Ubuntu and Debian it's dash.

And it turns out that dash won't do $'' quoting. My fault, I could have sworn this was in POSIX. (this is one of the reasons I prefer to write for a specific shell)

I would like if we could easily get something else to generate a U+FDD2, but I don't believe there's a good version without introducing a dependency, so we'll just let our printf do it:

echo (printf '\ufdd2foo') | string escape
# CHECK: \Xef\Xb7\X92foo

That's faho@b2ef44a. Passes on my system and Github Actions.

@dcermak
Copy link

dcermak commented Dec 5, 2023

That's faho@b2ef44a. Passes on my system and Github Actions.

This patch fixes the test failure in openSUSE Tumbleweed and Fedora Rawhide, thanks for creating it!

Will you create another dot release or should we patch this downstream?

@zanchey
Copy link
Member Author

zanchey commented Dec 5, 2023

I'll do a new release tonight.

@dcermak
Copy link

dcermak commented Dec 5, 2023 via email

@faho faho closed this as completed Dec 5, 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

5 participants