tests: Simplify comparison in rpc_blockchain.py #13924

pull domob1812 wants to merge 1 commits into bitcoin:master from domob1812:txoutset-test changing 1 files +4 −7

domob1812 commented at 2:42 PM on August 9, 2018: contributor

The test for gettxoutsetinfo in rpc_blockchain.py verifies that the result is the same as before after invalidating and reconsidering a block. The comparison has to exclude the disk_size field, though, as it is not deterministic.

Instead of comparing all the other fields for equality, this change explicitly removes the disk_size field and then compares the full objects. This makes the intent more explicit (compare everything except for disk_size, not compare just a given list of fields) and also the code simpler.

Simplify comparison in rpc_blockchain.py.

The test for gettxoutsetinfo in rpc_blockchain.py verifies that the
result is the same as before after invalidating and reconsidering a
block.  The comparison has to exclude the 'disk_size' field, though, as
it is not deterministic.

Instead of comparing all the other fields for equality, this change
explicitly removes the 'disk_size' field and then compares the full
objects.  This makes the intent more explicit (compare everything except
for disk_size, not compare just a given list of fields) and also the
code simpler.

1f87c372b5

domob1812 commented at 2:43 PM on August 9, 2018: contributor

A small negative side effect is that if the comparison fails, then you will just see both objects in the error message and have to find the exact diff yourself; previously you would immediately see which field had the diff. But that is mainly a problem of the assertion functions we use, and I think not so important here.

If you disagree, feel free to close the PR and keep the current field-for-field comparison.
fanquake added the label Tests on Aug 9, 2018
marcinja commented at 9:48 PM on August 10, 2018: contributor

ACK 1f87c37

This change (or at least the comment addition) makes it explicit why that field isn't checked. Someone reading the test right now might think it's incomplete or might not understand why a disk_size check is excluded.

When the assertion fails the output is still just a few lines long, so I don't see that as problematic.
MarcoFalke commented at 9:55 PM on August 10, 2018: member

utACK 1f87c372b521a1ff6970aca9df0ded839528ea20
sipa commented at 10:33 PM on August 10, 2018: member

utACK 1f87c372b521a1ff6970aca9df0ded839528ea20
MarcoFalke merged this on Aug 11, 2018
MarcoFalke closed this on Aug 11, 2018
MarcoFalke referenced this in commit 09ada21ca2 on Aug 11, 2018
domob1812 deleted the branch on Aug 11, 2018
jasonbcox referenced this in commit 2790a0a61e on Sep 23, 2020
PastaPastaPasta referenced this in commit 128fc681c2 on Feb 2, 2021
PastaPastaPasta referenced this in commit 99d24fca8f on Feb 3, 2021
PastaPastaPasta referenced this in commit e6bbb2f2b3 on Feb 4, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels