[tests] Better stderr testing #12755

jnewbery · 2018-03-22T13:52:58Z

Due to a merge conflict, this is now based on #10267. Please review that PR first!

Subset of #12379 now that parts of that PR have been merged.

#12362 was only observed when running the functional tests locally because:

by defatul libc logs to /dev/tty instead of stderr
the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail.

This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr:

commit Write stdout/stderr to datadir instead of temp file writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure.
commit Use LIBC_FATAL_STDERR=1 in tests ensures that libc failures are logged to stderr instead of the terminal.
commit Assert that bitcoind stdout is empty on shutdown asserts that stderr is empty on bitcoind shutdown.

annanay25 · 2018-03-23T07:35:34Z

test/functional/test_framework/test_node.py

Unable to replicate this bug on my machine, but the tests it fails on some machines is from these declarations. A small analysis -

The error reported is FileNotFoundError, however, checking the code for creating temp files in python:

def _mkstemp_inner(dir, pre, suf, flags): """Code common to mkstemp, TemporaryFile, and NamedTemporaryFile.""" names = _get_candidate_names() for seq in xrange(TMP_MAX): name = names.next() file = _os.path.join(dir, pre + name + suf) try: fd = _os.open(file, flags, 0600) _set_cloexec(fd) return (fd, _os.path.abspath(file)) except OSError, e: if e.errno == _errno.EEXIST: continue # try again raise raise IOError, (_errno.EEXIST, "No usable temporary file name found")

Such an error could not have originated from this function. So the OS created the file but its not visible? Reading the docs gives this -

Under Unix, the directory entry for the file is either not created at all or is removed immediately after the file is created. Other platforms do not support this; your code should not rely on a temporary file created using this function having or not having a visible name in the file system. ... Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms (it can be so used on Unix; it cannot on Windows NT or later).

jnewbery · 2018-04-30T19:36:02Z

Fixed the failing feature_config_args.py and rebased on master.

jnewbery · 2018-05-01T15:46:15Z

I've rebased this on top of #10267, since there's now a conflict between the two and it doesn't make sense for that PR to be held up on this.

This PR effectively takes the stderr checking in feature_includeconf.py added in 488f9477623d2c5eee65f0376b1f5db41d1f61a6 and makes it generic for all functional tests.

kallewoof

utACK 6a003e0c09c28dcac41323668e855e23abdeec7e

kallewoof · 2018-05-09T08:56:29Z

@jnewbery #10267 is now merged, so you can trim out the duplicate commits.

By default, libc will print fatal errors to /dev/tty instead of stderr. Adding the LIBC_FATAL_STDERR_ to the environment variables allows us to catch libc errors in stderr and test for them.

jnewbery · 2018-05-09T14:21:49Z

Thanks @kallewoof - rebased on master to trim out the #10267 commits

laanwj · 2018-05-09T14:37:34Z

silly linting error from travis:

10.78s$ if [ "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/lint-all.sh; fi
./test/functional/feature_includeconf.py:19:1: F401 'test_framework.test_framework.assert_equal' imported but unused
^---- failure generated from contrib/devtools/lint-python.sh

Allow bitcoind's stderr to be tested against a specified string on shutdown.

jnewbery · 2018-05-09T14:39:50Z

Thanks linter! Fixed.

laanwj · 2018-05-09T14:55:05Z

utACK beee49b

beee49b [tests] Allow stderr to be tested against specified string (John Newbery) e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery) c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery) Pull request description: **Due to a merge conflict, this is now based on #10267. Please review that PR first!** Subset of #12379 now that parts of that PR have been merged. #12362 was only observed when running the functional tests locally because: - by defatul libc logs to `/dev/tty` instead of stderr - the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail. This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr: - commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure. - commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal. commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown. Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048

@jnewbery

2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to #10267, and addresses #10267 (comment). ~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352

maflcko · 2018-05-23T15:16:31Z

post merge utACK beee49b

beee49b [tests] Allow stderr to be tested against specified string (John Newbery) e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery) c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery) Pull request description: **Due to a merge conflict, this is now based on bitcoin#10267. Please review that PR first!** Subset of bitcoin#12379 now that parts of that PR have been merged. bitcoin#12362 was only observed when running the functional tests locally because: - by defatul libc logs to `/dev/tty` instead of stderr - the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail. This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr: - commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure. - commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal. commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown. Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048

@jnewbery

…calls 2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm) c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm) Pull request description: This is a follow-up PR to bitcoin#10267, and addresses bitcoin#10267 (comment). ~~I am adding extra work for @jnewbery in bitcoin#12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~ Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352

fanquake added the Tests label Mar 22, 2018

jnewbery mentioned this pull request Mar 22, 2018

[WIP] Better stderr testing in functional tests #12379

Closed

annanay25 reviewed Mar 23, 2018

View reviewed changes

jnewbery force-pushed the better_stderr_testing branch from c23f804 to de4c006 Compare April 30, 2018 19:34

jnewbery force-pushed the better_stderr_testing branch 2 times, most recently from d3f9afe to 5480ee9 Compare April 30, 2018 20:19

jnewbery mentioned this pull request Apr 30, 2018

New -includeconf argument for including external configuration files #10267

Merged

jnewbery force-pushed the better_stderr_testing branch from 5480ee9 to 6a003e0 Compare May 1, 2018 15:44

kallewoof approved these changes May 7, 2018

View reviewed changes

kallewoof mentioned this pull request May 9, 2018

util: warn about ignored recursive -includeconf calls #13197

Merged

jnewbery added 2 commits May 9, 2018 09:56

[Tests] Write stdout/stderr to datadir instead of temp file.

c22ce8a

[Tests] Use LIBC_FATAL_STDERR_=1 in tests

e503671

By default, libc will print fatal errors to /dev/tty instead of stderr. Adding the LIBC_FATAL_STDERR_ to the environment variables allows us to catch libc errors in stderr and test for them.

jnewbery force-pushed the better_stderr_testing branch from 6a003e0 to 86254f9 Compare May 9, 2018 14:21

[tests] Allow stderr to be tested against specified string

beee49b

Allow bitcoind's stderr to be tested against a specified string on shutdown.

jnewbery force-pushed the better_stderr_testing branch from 86254f9 to beee49b Compare May 9, 2018 14:39

laanwj merged commit beee49b into bitcoin:master May 9, 2018

jnewbery deleted the better_stderr_testing branch May 9, 2018 14:57

laanwj mentioned this pull request May 9, 2018

[qa] Avoid printing to console during cache creation #13198

Merged

Bushstar mentioned this pull request May 10, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#328

Merged

HashUnlimited mentioned this pull request Sep 7, 2018

util: warn about ignored recursive -includeconf calls chaincoin/chaincoin#333

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[tests] Better stderr testing #12755

[tests] Better stderr testing #12755

Uh oh!

jnewbery commented Mar 22, 2018 •

edited

Loading

Uh oh!

annanay25 Mar 23, 2018

Uh oh!

jnewbery commented Apr 30, 2018

Uh oh!

jnewbery commented May 1, 2018

Uh oh!

kallewoof left a comment

Uh oh!

kallewoof commented May 9, 2018

Uh oh!

jnewbery commented May 9, 2018

Uh oh!

laanwj commented May 9, 2018

Uh oh!

jnewbery commented May 9, 2018

Uh oh!

laanwj commented May 9, 2018

Uh oh!

maflcko commented May 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[tests] Better stderr testing #12755

[tests] Better stderr testing #12755

Uh oh!

Conversation

jnewbery commented Mar 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

annanay25 Mar 23, 2018

Choose a reason for hiding this comment

Uh oh!

jnewbery commented Apr 30, 2018

Uh oh!

jnewbery commented May 1, 2018

Uh oh!

kallewoof left a comment

Choose a reason for hiding this comment

Uh oh!

kallewoof commented May 9, 2018

Uh oh!

jnewbery commented May 9, 2018

Uh oh!

laanwj commented May 9, 2018

Uh oh!

jnewbery commented May 9, 2018

Uh oh!

laanwj commented May 9, 2018

Uh oh!

maflcko commented May 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jnewbery commented Mar 22, 2018 •

edited

Loading