[WIP] Better stderr testing in functional tests #12379

1. 
   jnewbery commented at 4:03 pm on February 7, 2018: member

   (WIP because if we’re not careful this could result in a lot of false +ves. Should be tested on different platforms to verify portability).

   #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 rename TestNode to TestP2PConn in tests is not necessary in this PR, but it removes a name conflict in the functional tests (which makes it difficult to see all the places that TestNode is being called)
   • commit Move assert_start_raises_init_error method to TestNode moves a method from test_framework to test_node (in general, methods which only affect a single node should be methods in TestNode).
   • 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 Update assert_start_raises_init_eror() to test for an exact m… changes assert_start_raises_init_eror() to match on an exact regex instead of a substring (with due apologies to @laanwj for #12302).
   • commit Assert that bitcoind stdout is empty on shutdown asserts that stderr is empty on bitcoind shutdown.
2. scripted-diff: rename TestNode to TestP2PConn in tests

   Several test scripts define a subclass of P2PInterface called TestNode.
   This commit renames those to TestP2PConn since we already have a
   TestNode class in the test framework.
   
   -BEGIN VERIFY SCRIPT-
   sed -i s/TestNode/TestP2PConn/ test/functional/*py test/functional/test_framework/comptool.py
   _END VERIFY SCRIPT-

   7dcaa1a979
3. [Tests] Move assert_start_raises_init_error method to TestNode 55e4c2b9f2
4. [Tests] Write stdout/stderr to datadir instead of temp file. 9ce0dc42cb
5. [Tests] Use LIBC_FATAL_STDERR_=1 in tests

   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.

   34e115a938
6. [Tests] Update assert_start_raises_init_eror() to test for an exact match in stdout b5b917fe02
7. laanwj added the label Tests on Feb 7, 2018
8. in test/functional/test_framework/test_node.py:200 in f70e7997d2 outdated

   195+                # Check stderr for expected message
   196+                if expected_msg is not None:
   197+                    log_stderr.seek(0)
   198+                    stderr = log_stderr.read().decode('utf-8')
   199+                    if re.fullmatch(expected_msg + '\n', stderr) is None:
   200+                        raise AssertionError('Expected stdout "{}" does not match stdout:\n"{}'.format(expected_msg, stderr))
   

   ---

   ---

   

   MarcoFalke commented at 10:49 pm on February 7, 2018:

   Nit: Should say stderr in the string?
9. 
   MarcoFalke commented at 10:50 pm on February 7, 2018: member

   The scripted diff should really go into a separate pull
10. [Tests] Assert that bitcoind stderr is empty on shutdown a13e6145a7
11. jnewbery force-pushed on Feb 8, 2018
12. 
    laanwj commented at 7:41 pm on February 8, 2018: member

    Concept ACK
13. 
    randolf commented at 2:34 am on February 15, 2018: contributor

    Concept ACK.
14. 
    Sjors commented at 5:57 pm on February 20, 2018: member

    I ran test_runner.py on MacOS 10.13.3. wallet_encryption.py, wallet_multiwallet.py and wallet_multiwallet.py --usecli failed. I ran them again individually and they still failed. See logs. The first test fails on master too (see #12375), but those other two pass on master (ffc6e48b2).
15. 
    jnewbery commented at 7:04 pm on March 19, 2018: member

    I’ve pulled out the first commit into #12728, and the assert_start_raises_init_error() commits have been pulled into #12718.

    I’ll leave this PR open for now, but will close and replace with a separate PR for the libc/stdout changes once those other two PRs are merged.

16. laanwj referenced this in commit 185d48473e on Mar 22, 2018
17. 
    MarcoFalke commented at 9:52 am on March 22, 2018: member

    Needs rebase
18. 
    jnewbery commented at 1:53 pm on March 22, 2018: member

    Parts of this PR have been merged in other PRs. Closing this in favour of #12755 which contains only the parts which have not yet been merged.
19. jnewbery closed this on Mar 22, 2018

    ---

20. laanwj referenced this in commit 612ba35ab1 on May 9, 2018
21. PastaPastaPasta referenced this in commit 62ece1f1ff on Sep 27, 2020
22. PastaPastaPasta referenced this in commit 7a880f29df on Oct 22, 2020
23. TheArbitrator referenced this in commit 92bb93cef0 on Jun 7, 2021
24. UdjinM6 referenced this in commit a57a82be1a on Jun 18, 2021
25. UdjinM6 referenced this in commit 9e9cd7c035 on Jun 24, 2021
26. UdjinM6 referenced this in commit 37b4656d23 on Jun 26, 2021
27. UdjinM6 referenced this in commit 18e8ca4fa1 on Jun 26, 2021
28. UdjinM6 referenced this in commit cc38451955 on Jun 28, 2021
29. DrahtBot locked this on Sep 8, 2021

Contributors
jnewbery MarcoFalke laanwj randolf Sjors

Labels
Tests