This idea came up in conversation last night and two people told me it was terrible, so I was curious to try a wider audience. Idea would be to have an assert_debug_log(node, regex) function in python.
Example use:
0self.nodes[0].p2p.send_txs([tx_orphan_invalid])
1with assert_debug_log(self.nodes[0], "invalid orphan tx %s" % txid(tx_orphan_invalid)):
2 self.nodes[0].p2p.send_txs([tx_orphan_parent])
Possible implementation:
0[@contextmanager](/bitcoin-bitcoin/contributor/contextmanager/)
1def assert_debug_log(node, pattern, **other_options):
2 prev_size = node.rpc.flush_debug_log()
3 try:
4 yield
5 finally:
6 new_size = node.rpc.flush_debug_log()
7 with open(node.datadir_path("debug.log")) as log:
8 log.seek(prev_size)
9 if not re.match(pattern, log.read(new_size - prev_size)):
10 raise AssertionError
Advantages:
- Log asserts could make existing tests simpler, more robust, and more readable because they would allow tests to check conditions they care about directly instead of reverse engineering node behaviour with counters and flags and inferences from p2p messages and disconnects.
- Log asserts could improve test coverage. Lots of PRs are currently created that fix bugs and change behavior and have 0 accompanying tests because they would be difficult to write in the c++ test framework, and unreliable in the python framework.
Disadvantages:
- Log asserts could be overused to check conditions that would be better to interrogate more directly.
- Log asserts could be overused to check behaviours that shouldn’t be checked at all because they are implementation specific.
- Log asserts could be overused to check trivial things not worth checking given the relative expense (performance, code verbosity, cost of maintaining fragile string comparisons).
- Log asserts could encourage writing integration tests when unit tests would be more appropriate.
- Log asserts could encourage writing white box tests when black box tests would be more appropriate.
- Log asserts could encourage using the python test framework instead of the c++ framework.
Conclusion:
I think assert_debug_log could simplify existing tests, fill a gap in the testing framework, and help move closer to a world where PRs that change internal bitcoin behaviour more typically come with tests. I think assert_debug_log could be overused, but in practice this could be avoided with code review and documentation. More abstract concerns like white box vs black box, unit vs functional, and python vs c++ testing tend to come off a bit superstitious and cargo-culty to me so I’m not the best person to respond to them, but I wouldn’t discourage other discussion in such terms.
