Some, but not all, backward compatibility tests fail for me and it seems useless to run old release binaries under valgrind anyway.
Can be tested by running test/functional/feature_txindex_compatibility.py --valgrind --timeout-factor=10 with and without this PR.
— The previous version of this PR disabled these test entirely under valgrind. The current version does run the test, but starts the old binaries without valgrind.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
No conflicts as of last run.
Example failure:
0$ test/functional/feature_txindex_compatibility.py --valgrind --timeout-factor=4
12023-03-08T14:43:48.054000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_c2c_8427
22023-03-08T14:43:49.852000Z TestFramework (ERROR): Assertion failed
3Traceback (most recent call last):
4 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 559, in start_nodes
5 node.wait_for_rpc_connection()
6 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_node.py", line 232, in wait_for_rpc_connection
7 'bitcoind exited with status {} during initialization'.format(self.process.returncode)))
8test_framework.test_node.FailedToStartError: [node 0] bitcoind exited with status 1 during initialization
9
10During handling of the above exception, another exception occurred:
11
12Traceback (most recent call last):
13 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 133, in main
14 self.setup()
15 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 297, in setup
16 self.setup_network()
17 File "test/functional/feature_txindex_compatibility.py", line 39, in setup_network
18 self.start_nodes()
19 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 562, in start_nodes
20 self.stop_nodes()
21 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 577, in stop_nodes
22 node.stop_node(wait=wait, wait_until_stopped=False)
23 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_node.py", line 344, in stop_node
24 self.stop()
25 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_node.py", line 190, in __getattr__
26 assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
27AssertionError: [node 0] Error: no RPC connection
282023-03-08T14:43:49.903000Z TestFramework (INFO): Stopping nodes
29Traceback (most recent call last):
30 File "test/functional/feature_txindex_compatibility.py", line 91, in <module>
31 TxindexCompatibilityTest().main()
32 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 157, in main
33 exit_code = self.shutdown()
34 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 313, in shutdown
35 self.stop_nodes()
36 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 577, in stop_nodes
37 node.stop_node(wait=wait, wait_until_stopped=False)
38 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_node.py", line 344, in stop_node
39 self.stop()
40 File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_node.py", line 190, in __getattr__
41 assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
42AssertionError: [node 0] Error: no RPC connection
43[node 2] Cleaning up leftover process
44[node 1] Cleaning up leftover process
45[node 0] Cleaning up leftover process
test/functional/mempool_compatibility.py etc?
can't run this test under valgrind message is not useful. I assume the first questions someone will have are going to be “Why can’t I run this test under Valgrind?”, “Does it need fixing then?” etc.
I disabled all tests that use previous releases under valgrind. Also tried a different error message.
“Does it need fixing then?”
I added a code comment and some more context to the commit to help answer that, if anyone does want to run these under valgrind.
24@@ -25,6 +25,8 @@ def set_test_params(self):
25
26 def skip_test_if_missing_module(self):
27 self.skip_if_no_previous_releases()
28+ # Not all release binaries work with valgrind.
skip_if_no_previous_releases function?
(future) tests we want to skip under valgrind
Let’s not worry about unwritten tests, that we apparently might not want to test properly for some arbitrary reason.
I was also thinking existing tests, e.g. I haven’t been able to run valgrind with BDB configured (didn’t try very hard).
It definitely works. The native valgrind CI job is already configured to run with BDB.
Conversely, since some tests do work, maybe someone wants to enable one later.
If you want to make it more fine grained, it could make sense to move the check to the test_node and condition it on the version number?
BDB
Good point. I also can’t run --valgrind with BDB on my system. Which raises the question how many settings we want --valgrind to be supported under.
Which raises the question how many settings we want –valgrind to be supported under.
I thnk currently the only realistic place we can fully support Valgrind, is within the CI system, where things like BDB are working, and we can more easily control settings and suppressions. Outside of that, things may work fine, or may not at all, depending on the system where it is being run.
This is unnecessary and caused test failures. The backward
compatibility tests are meant to find regressions in the
current codebase, not to detect bugs in older releases.