tests: Improve stderr validation in test_runner.py #32183

1. 
   GarmashAlex commented at 9:54 am on April 1, 2025: none

   This PR implements the improvement suggested in a TODO comment in test/util/test_runner.py. It adds validation that stderr is empty when no errors are expected in test cases.

   The change adds a check that verifies stderr is empty when no error_txt is specified in the test object, with a special exception for bitcoin-tx running under Wine, which was mentioned in the original TODO comment.

   This improvement helps catch unexpected error messages that might otherwise go unnoticed during testing, making the test framework more robust.

   The implementation:

   1. Checks if stderr contains output when no error is expected
   2. Makes an exception for the known case of bitcoin-tx running under Wine
   3. Raises an exception with a detailed error message if unexpected stderr output is found
   4. Removes the TODO comment as the task is now completed

   This change improves test coverage by ensuring that tests don’t silently pass when they produce unexpected error output.

2. tests: Improve stderr validation in test_runner.py

   This commit implements the improvement suggested in a TODO comment by adding
   validation that stderr is empty when no errors are expected in test cases.
   
   The change:
   - Adds a check that verifies stderr is empty when no error_txt is specified
   - Makes an exception for bitcoin-tx running under Wine
   - Removes the TODO comment as the task is now completed

   364133bc64
3. 
   DrahtBot commented at 9:54 am on April 1, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32183.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

4. DrahtBot added the label Tests on Apr 1, 2025
5. in test/util/test_runner.py:76 in 364133bc64

   72@@ -73,7 +73,7 @@ def bctest(testDir, testObj, buildenv):
   73     are not as expected. Error is caught by bctester() and reported.
   74     """
   75     # Get the exec names and arguments
   76-    execprog = os.path.join(buildenv["BUILDDIR"], "bin", testObj["exec"] + buildenv["EXEEXT"])
   77+    execprog = os.path.join(buildenv["BUILDDIR"], "src", testObj["exec"] + buildenv["EXEEXT"])
   

   ---

   ---

   

   maflcko commented at 10:12 am on April 1, 2025:

   @hebasto Is there a reason why

   0cmake/tests.cmake:7:    COMMAND ${CMAKE_COMMAND} -E env BITCOINUTIL=$<TARGET_FILE:bitcoin-util> BITCOINTX=$<TARGET_FILE:bitcoin-tx> ${PYTHON_COMMAND} ${PROJECT_BINARY_DIR}/test/util/test_runner.py
   

   sets env vars at all, when this fallback should already work fine and when the autotools-based build system did not set them either?

   Ref:

   0check-local: check-unit
   1if BUILD_BITCOIN_TX	[@echo](/bitcoin-bitcoin/contributor/echo/) "Running test/util/test_runner.py..."
   2	$(PYTHON) $(top_builddir)/test/util/test_runner.py
   3endif	[@echo](/bitcoin-bitcoin/contributor/echo/) "Running test/util/rpcauth-test.py..."
   4	$(PYTHON) $(top_builddir)/test/util/rpcauth-test.py
   5...
   

   I am asking, because removing the env vars would likely detect the bug introduced in this pull.

   ---

   

   hebasto commented at 10:26 am on April 22, 2025:

   This is intended to handle multi-config generator, which create per-config subdirectories:

   0$ cmake -B build-mc -G "Ninja Multi-Config"
   1$ cmake --build build-mc
   2$ ctest --test-dir build-mc -R util_test_runner
   

   ---

   

   hebasto commented at 10:50 am on April 22, 2025:

   @maflcko

   I believe #32324 should address your concerns.

6. in test/util/test_runner.py:159 in 364133bc64

   155@@ -156,14 +156,18 @@ def bctest(testDir, testObj, buildenv):
   156     if "error_txt" in testObj:
   157         want_error = testObj["error_txt"]
   158         # Compare error text
   159-        # TODO: ideally, we'd compare the strings exactly and also assert
   

   ---

   ---

   

   fanquake commented at 11:30 am on April 1, 2025:

   This is still only partially removing the TODO (previously #31966).
7. 
   fanquake commented at 4:03 pm on April 22, 2025: member

   Closing in favour of #32327.
8. fanquake closed this on Apr 22, 2025

   ---

9. GarmashAlex deleted the branch on Apr 28, 2025

Contributors
GarmashAlex DrahtBot maflcko hebasto fanquake

Labels
Tests