tests: Fix fs_tests for unknown locales #17086

carnhofdaki · 2019-10-09T14:51:43Z

Fix by removing "L" as suggested by meeDamian in
#14948 (comment)

# all in .../bitcoin/src/test
$ uname -m
x86_64
$ export LC_ALL=randomnonexistentlocale
$ ./test_bitcoin
Running 369 test cases...
unknown location(0): fatal error: in "fs_tests/fsbridge_fstream": boost::system::system_error: boost::filesystem::path codecvt to string: error
test/fs_tests.cpp(13): last checkpoint: "fsbridge_fstream" test entry

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

After the patch is applied, the same test under the same conditions runs fine.

$ export LC_ALL=randomnonexistentlocale
$ ./test_bitcoin
Running 369 test cases...

*** No errors detected

Co-Authored-By: [email protected]

fanquake · 2019-10-09T15:02:40Z

@carnhofdaki please remove any @mentions from your commit message. I have removed the one in your PR description. We don't include them in either as they just result in spam. If you'd like to attribute the change you could use the Co-Authored-By: name <email> syntax in your commit.

laanwj · 2019-10-09T15:13:44Z

ACK (after fanquake's comment addressed)

Comment #14948 (comment)
Turns out this is the only place a L" string is used (besides a windows-specific workaround in src/util/system.cpp, irrelevant to this). No need to test this if we're not using it in the application.

carnhofdaki · 2019-10-10T07:15:47Z

@fanquake thank you for comment. Done. Is it good now?

meeDamian · 2019-10-12T21:01:35Z

please remove any @mentions from your commit message. I have removed the one in your PR description. We don't include them in either as they just result in spam.

💔, but 💯 agree.

This change fixes builds for me, looking forward to merge!

Fix by removing "L" as suggested by meeDamian in bitcoin#14948 (comment) Co-Authored-By: [email protected]

laanwj · 2019-10-15T09:33:31Z

ACK d48f664

Hrmph, we should probably fix the false-positive in github-merge.py for full mail addresses. Those don't get notified (afaik).

d48f664 tests: Fix fs_tests for unknown locales (Daki Carnhof) Pull request description: Fix by removing "L" as suggested by meeDamian in #14948 (comment) ``` # all in .../bitcoin/src/test $ uname -m x86_64 $ export LC_ALL=randomnonexistentlocale $ ./test_bitcoin Running 369 test cases... unknown location(0): fatal error: in "fs_tests/fsbridge_fstream": boost::system::system_error: boost::filesystem::path codecvt to string: error test/fs_tests.cpp(13): last checkpoint: "fsbridge_fstream" test entry *** 1 failure is detected in the test module "Bitcoin Core Test Suite" ``` After the patch is applied, the same test under the same conditions runs fine. ``` $ export LC_ALL=randomnonexistentlocale $ ./test_bitcoin Running 369 test cases... *** No errors detected ``` Co-Authored-By: [email protected] ACKs for top commit: laanwj: ACK d48f664 Tree-SHA512: a9910252b8ce6a05cab5530874549c2999ca2c28e835fc18aa8e5468fb417bd7d245864ec71d9233dd53e02940a9f0691b247430257f27eb0d7c20745d1c846d

hebasto · 2019-10-15T16:28:14Z

Post-merge ACK. Tested on ODROID-HC1:

$ gcc --version
gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ locale
LANG=
LANGUAGE=
LC_CTYPE="POSIX"
LC_NUMERIC="POSIX"
LC_TIME="POSIX"
LC_COLLATE="POSIX"
LC_MONETARY="POSIX"
LC_MESSAGES="POSIX"
LC_PAPER="POSIX"
LC_NAME="POSIX"
LC_ADDRESS="POSIX"
LC_TELEPHONE="POSIX"
LC_MEASUREMENT="POSIX"
LC_IDENTIFICATION="POSIX"
LC_ALL=

Could this PR be backported to 0.19?

carnhofdaki · 2019-10-16T07:55:48Z

@hebasto sure! See #17158

@MarcoFalke What is the process for backports? (In case I did something wrong)

Fix by removing "L" as suggested by meeDamian in bitcoin#14948 (comment) Co-Authored-By: [email protected] Github-Pull: bitcoin#17086 Rebased-From: d48f664

bd9d40d tests: Fix fs_tests for unknown locales (Daki Carnhof) Pull request description: Backporting to `0.19` as suggested in #17086 (comment) Fix by removing "L" as suggested by meeDamian in #14948 (comment) Co-Authored-By: [email protected] Top commit has no ACKs. Tree-SHA512: cb73c475560d156034d240c77dfd704526cfb148bcecf302079f1f9b6984117da71f018e1c70a165caed90be48482cb9c4939b001477a44f562fc0c11cb6ede7

fanquake · 2019-10-19T17:52:56Z

Backported in #17158.

Fix by removing "L" as suggested by meeDamian in bitcoin#14948 (comment) Co-Authored-By: [email protected] Github-Pull: bitcoin#17086 Rebased-From: d48f664

Sjors · 2019-11-27T13:31:04Z

This fixed the test suite for me on an armv7 device.

Summary: Backport of core [[bitcoin/bitcoin#17086 | PR17086]]. Test Plan: ninja check Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5687

d48f664 tests: Fix fs_tests for unknown locales (Daki Carnhof) Pull request description: Fix by removing "L" as suggested by meeDamian in bitcoin#14948 (comment) ``` # all in .../bitcoin/src/test $ uname -m x86_64 $ export LC_ALL=randomnonexistentlocale $ ./test_bitcoin Running 369 test cases... unknown location(0): fatal error: in "fs_tests/fsbridge_fstream": boost::system::system_error: boost::filesystem::path codecvt to string: error test/fs_tests.cpp(13): last checkpoint: "fsbridge_fstream" test entry *** 1 failure is detected in the test module "Bitcoin Core Test Suite" ``` After the patch is applied, the same test under the same conditions runs fine. ``` $ export LC_ALL=randomnonexistentlocale $ ./test_bitcoin Running 369 test cases... *** No errors detected ``` Co-Authored-By: [email protected] ACKs for top commit: laanwj: ACK d48f664 Tree-SHA512: a9910252b8ce6a05cab5530874549c2999ca2c28e835fc18aa8e5468fb417bd7d245864ec71d9233dd53e02940a9f0691b247430257f27eb0d7c20745d1c846d

merge bitcoin#14209, bitcoin#13878, bitcoin#14192, bitcoin#17086: logging and filesystem updates

fanquake added the Tests label Oct 9, 2019

carnhofdaki force-pushed the cdk/fix_fs_tests branch from 4c58943 to e6208c8 Compare October 9, 2019 14:58

fanquake added the Waiting for author label Oct 9, 2019

carnhofdaki force-pushed the cdk/fix_fs_tests branch from e6208c8 to 4375323 Compare October 10, 2019 07:15

laanwj changed the title ~~tests: Fix fs_tests for unknows locales~~ tests: Fix fs_tests for unknown locales Oct 14, 2019

carnhofdaki force-pushed the cdk/fix_fs_tests branch from 4375323 to 000198d Compare October 15, 2019 09:16

tests: Fix fs_tests for unknown locales

d48f664

Fix by removing "L" as suggested by meeDamian in bitcoin#14948 (comment) Co-Authored-By: [email protected]

carnhofdaki force-pushed the cdk/fix_fs_tests branch from 000198d to d48f664 Compare October 15, 2019 09:22

laanwj merged commit d48f664 into bitcoin:master Oct 15, 2019

carnhofdaki deleted the cdk/fix_fs_tests branch October 15, 2019 09:39

maflcko added Needs backport (0.19) and removed Waiting for author labels Oct 15, 2019

maflcko added this to the 0.19.0 milestone Oct 15, 2019

carnhofdaki mentioned this pull request Oct 16, 2019

tests: Fix fs_tests for unknown locales #17158

Merged

fanquake removed the Needs backport (0.19) label Oct 19, 2019

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

kwvg mentioned this pull request Jun 24, 2021

merge bitcoin#14209, #13878, #14192, #17086: logging and filesystem updates dashpay/dash#4199

Merged

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Jun 25, 2021

Merge pull request #4199 from kittywhiskers/fstreamWrapper

e887596

merge bitcoin#14209, bitcoin#13878, bitcoin#14192, bitcoin#17086: logging and filesystem updates

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tests: Fix fs_tests for unknown locales #17086

tests: Fix fs_tests for unknown locales #17086

Uh oh!

carnhofdaki commented Oct 9, 2019 •

edited

Loading

Uh oh!

fanquake commented Oct 9, 2019 •

edited

Loading

Uh oh!

laanwj commented Oct 9, 2019 •

edited

Loading

Uh oh!

carnhofdaki commented Oct 10, 2019

Uh oh!

meeDamian commented Oct 12, 2019

Uh oh!

laanwj commented Oct 15, 2019

Uh oh!

hebasto commented Oct 15, 2019

Uh oh!

carnhofdaki commented Oct 16, 2019

Uh oh!

fanquake commented Oct 19, 2019

Uh oh!

Sjors commented Nov 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tests: Fix fs_tests for unknown locales #17086

tests: Fix fs_tests for unknown locales #17086

Uh oh!

Conversation

carnhofdaki commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carnhofdaki commented Oct 10, 2019

Uh oh!

meeDamian commented Oct 12, 2019

Uh oh!

laanwj commented Oct 15, 2019

Uh oh!

hebasto commented Oct 15, 2019

Uh oh!

carnhofdaki commented Oct 16, 2019

Uh oh!

fanquake commented Oct 19, 2019

Uh oh!

Sjors commented Nov 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

carnhofdaki commented Oct 9, 2019 •

edited

Loading

fanquake commented Oct 9, 2019 •

edited

Loading

laanwj commented Oct 9, 2019 •

edited

Loading