[Tests] Add mempool_limit.py test #7153

pull jonasschnelli wants to merge 2 commits into bitcoin:master from jonasschnelli:2015/12/mempool-test changing 4 files +116 −49

jonasschnelli commented at 7:37 AM on December 2, 2015: contributor

This will test mempools tx eviction (-maxmempool) together with the new conflict handling (#7105)
[Tests] Add mempool_limit.py test 110ff1142c
jonasschnelli added the label Tests on Dec 2, 2015
in qa/rpc-tests/mempool_limit.py:None in 110ff1142c outdated
```
  12 | +
  13 | +    def satoshi_round(self, amount):
  14 | +        return  Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
  15 | +
  16 | +    def __init__(self):
  17 | +        # Some pre-processing to create a bunch of OP_RETURN txouts to insert into transactions we create
```
MarcoFalke commented at 7:40 AM on December 2, 2015:

If you copied this code, DRY?

jonasschnelli commented at 7:44 AM on December 2, 2015:

Yes. I think this code part is in maxuploadtarget.py, prioritise_transaction.py and pruning.py. Someone should refactor that once. I have original taken this from a test that @sdaftuar wrote.

MarcoFalke commented at 7:47 AM on December 2, 2015:

Mind to do it in this PR like 826079ef34906539010abe63537e7ceafedbff01?

MarcoFalke commented at 7:47 AM on December 2, 2015:

It's enough to review this method once and not every time a pull tester PR is opened.
jonasschnelli force-pushed on Dec 3, 2015
jonasschnelli commented at 10:05 AM on December 3, 2015: contributor

Nits addresses: refactored the shared functions (moved to util.py).
[Tests] Refactor some shared functions 7632cf689a
in qa/rpc-tests/mempool_limit.py:None in 5d5ada9460 outdated
```
   8 | +from test_framework.test_framework import BitcoinTestFramework
   9 | +from test_framework.util import *
  10 | +
  11 | +class MempoolLimitTest(BitcoinTestFramework):
  12 | +
  13 | +    def __init__(self):
```
MarcoFalke commented at 11:35 AM on December 3, 2015:

Can you do the same here?

def __init__(self): self.txouts = create_many_txouts()
jonasschnelli commented at 12:05 PM on December 3, 2015:

This part needs to be refactored between 4 existing tests. This PRs intent is not to refactor the test framework instead it should add a mempool limit/eviction test.

MarcoFalke commented at 12:18 PM on December 3, 2015:

Consider it a NIT but I don't like having the same code in 4 locations. You are guaranteed to get a bug out of that some day. (We already had it with other rpc tests)

laanwj commented at 1:01 PM on December 3, 2015:

Having the same code in 4 locations only makes sense if it is expected to diverge again later. Over-eager de-duplication can result in more complex code in such cases.

I don't think that's the case here. But I also agree with @jonasschnelli that refactoring is not required in a PR that adds a test.

in qa/rpc-tests/mempool_limit.py:None in 5d5ada9460 outdated

  28 | +            self.txouts = self.txouts + script_pubkey
  29 | +                    
  30 | +    def setup_network(self):
  31 | +        self.nodes = []
  32 | +        self.nodes.append(start_node(0, self.options.tmpdir, ["-maxmempool=5", "-spendzeroconfchange=0", "-debug"]))
  33 | +        self.nodes.append(start_node(1, self.options.tmpdir, []))

MarcoFalke commented at 12:15 PM on December 3, 2015:

Nit: Is it required to have two nodes?

jonasschnelli commented at 12:26 PM on December 3, 2015:

Good point! Is was only required because we took an address from the 2nd node (but can also be an address from node[0]). Thanks. Fixed and force pushed.

jonasschnelli force-pushed on Dec 3, 2015
MarcoFalke commented at 12:32 PM on December 3, 2015: member

utACK 7632cf6. Runs for 4 secs, so should be ok to run every time.
laanwj merged this on Dec 22, 2015
laanwj closed this on Dec 22, 2015
laanwj referenced this in commit 97d83739db on Dec 22, 2015
laanwj commented at 1:07 PM on December 22, 2015: member

utACK https://github.com/jonasschnelli/bitcoin/commit/7632cf689a9b959dd7a059b8b4a04761a4bc6e6a
jonasschnelli referenced this in commit 9ef7c54ef0 on Dec 22, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7105 Keep track of explicit wallet conflicts instead of using mempool