Now that wallet filenames are properly quoted when used for rpc (#13823), we can add some unicode symbols to the test_runner path. Thus, the "extern" wallet that uses a full path has a unicode symbol in its name.
Should add unicode coverage to
listwalletswallet.getwalletinfo(un)loadwallet
Concept ACK
I feel like this is an opportunity to use ₿ somewhere.
Concept ACK stronger ACK if you mine the commit hash
<!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:
.travis/ subject to shellcheck by scravy)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
utACK fa5a3f51cc4b3dae2f134fae1745f392ca6e5e47
utACK fa5a3f51cc4b3dae2f134fae1745f392ca6e5e47 – the macos build failed with a timeout, maybe restart the build?
utACK 5e1777777790e855a9f3c8604208bc9bd6c8c99f. You have seven "7" in your commit hash.
utACK 5e1777777790e855a9f3c8604208bc9bd6c8c99f
💜-ACK 5e1777777790e855a9f3c8604208bc9bd6c8c99f
This seems to be causing travis errors:
Traceback (most recent call last):
File "test/functional/test_runner.py", line 616, in <module>
main()
File "test/functional/test_runner.py", line 233, in main
os.makedirs(tmpdir)
File "/usr/lib/python3.6/os.py", line 220, in makedirs
mkdir(name, mode)
UnicodeEncodeError: 'ascii' codec can't encode character '\u20bf' in position 17: ordinal not in range(128)
https://travis-ci.org/bitcoin/bitcoin/jobs/402357380#L3946
I see same error locally if I run LC_ALL=C test/functional/test_runner.py, but perhaps this is expected.
@ryanofsky The travis error is due to a travis bug, where it wouldn't use the most recent travis yaml when a build is reset/rerun.
Locally, you'd have to pick a language setting that is UTF-8 or set LC_ALL=C.UTF-8
This is solved in https://github.com/bitcoin/bitcoin/pull/13863
I like this and think it's fun... but is it possible we can revert this? It breaks clipboard copy-paste behavior on my system so it makes it a bit annoying to copy the directiory to examine the directory on failed test.
It's a minor annoyance, but idk if it's worth it?
@JeremyRubin We should absolutely test wallet behaviour for non-ascii directory and file paths. Especially given that it was broken in the past and some Bitcoin Core users have a non-ascii name.
Ah I guess I mean just the test runner part.
I'm fine with it being non ASCII, but copy paste highlighting treats the Emojis as whitespace on my system which makes it a bit more painful to inspect the tmp dir when debugging.
On Thu, Nov 7, 2019, 6:11 AM MarcoFalke notifications@github.com wrote:
@JeremyRubin https://github.com/JeremyRubin We should absolutely test wallet behaviour for non-ascii directory and file paths. Especially given that it was broken in the past and some Bitcoin Core users have a non-ascii name.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/13859?email_source=notifications&email_token=AAGYN6Y5KFF6R4QEPPK3ON3QSQOX5A5CNFSM4FNU3AR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDMQXLA#issuecomment-551095212, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGYN6ZLWGCNPJKXSAVAXF3QSQOX5ANCNFSM4FNU3ARQ .
The test runner path defines the root dir for the temp test dirs, and some of our wallets use absolute paths that are not in the datadir or wallet dir. So the root dir is the only place where an emoji could be injected to make the path non-ascii.
Have you reported the clipboard bug upstream?
Have you reported the clipboard bug upstream?
I have the same problem with gnome terminal. They are really stubborn about their UI and in the past removed the ability to customize this behavior:
https://bugzilla.gnome.org/show_bug.cgi?id=730632
But maybe there is a way to customize it again:
Though I think my real solution should just be to get a better terminal. Or maybe github could shadowban Marco's emojis so only he can be amused by them and everybody else will just wonder why he's so happy all the time.
Maybe we could add an OPT_OUT_OF_EMOJI environment variable that the test_runner checks before using emojis? The environment variable could be set in your .bashrc.