test: Check expected_stderr after stop #28028

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2307-test-stderr- changing 2 files +15 −14

maflcko commented at 12:17 PM on July 4, 2023: member

This fixes a bug where stderr wasn't checked for the shutdown sequence.

Fix that by waiting for the shutdown to finish and then check stderr.
DrahtBot commented at 12:17 PM on July 4, 2023: contributor



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK theStack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Jul 4, 2023
maflcko force-pushed on Jul 4, 2023
DrahtBot added the label CI failed on Jul 4, 2023
fanquake requested review from theStack on Jul 4, 2023
maflcko force-pushed on Jul 4, 2023
DrahtBot removed the label CI failed on Jul 6, 2023

Type	Reviewers
ACK	theStack

in test/functional/test_framework/test_node.py:358 in 222280fcef outdated

 362 | -        self.stdout.close()
 363 | -        self.stderr.close()
 364 | -
 365 |          del self.p2ps[:]
 366 |  
 367 | +        assert not expected_stderr or wait_until_stopped  # Must wait to check stderr

theStack commented at 11:18 AM on July 9, 2023:

nit, logically equivalent, but likely less ambiguous for the reader's eyes:

        assert (not expected_stderr) or wait_until_stopped  # Must wait to check stderr

maflcko commented at 1:59 PM on July 10, 2023:

thx, done

theStack commented at 11:24 AM on July 9, 2023: contributor

Code looks good to me, will test locally tomorrow.
test: Check expected_stderr after stop faf902858d
maflcko force-pushed on Jul 10, 2023
theStack approved
theStack commented at 4:40 PM on July 10, 2023: contributor

ACK faf902858d38150caa8991b0ab9d7cfee2905684

Makes sense to do the stderr check at the very end after shutdown.
fanquake merged this on Jul 11, 2023
fanquake closed this on Jul 11, 2023
maflcko deleted the branch on Jul 11, 2023
sidhujag referenced this in commit 4e2f49f42b on Jul 12, 2023
bitcoin locked this on Dec 5, 2024

Contributors

Labels

test: Check expected_stderr after stop #28028

Reviews