test: refactor: dedup mempool_package_limits.py subtests via decorator #27350
+51
−75
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee 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. |
glozow
reviewed
Mar 28, 2023
This avoids having to convert from BTC to Sats and needs less imports. Also specify the tx's target size in vsize rather than in weight, which allows us to specify the fee-rate by a simple multiplication, rather than having another magic number for it.
0ed2f03 to
e669833
Compare
glozow
reviewed
Mar 29, 2023
Member
|
ACK e669833 |
| # "package-mempool-limits" for each tx | ||
| # 4) after mining a block, clearing the pre-submitted transactions from mempool, | ||
| # check that submitting the created package succeeds | ||
| def check_package_limits(func): |
Member
There was a problem hiding this comment.
Nice 💯
Just a couple of nit minor suggestions to improve the readability of the code:
Add a blank line after the first assertion assert_equal(0, node.getmempoolinfo()["size"])
Add a blank line after the line where package_hex is assigned to func(self, *args, **kwargs)
Separating the operations as written in the decorator comment, also goes well with PEP9 guidelines.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The subtests in the functional test mempool_package_limits.py all follow the same pattern:
testmempoolaccepton the package hex fails with a "package-mempool-limits" error on each tx resultNote that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so this might be a nice opportunity to deduplicate code by using a newly introduced decorator which executes the necessary before and after the essential part of the subtest. This also makes it easier to add new subtests without having to copy-paste those parts once again.
In addition, the first commit switches the fee unit from BTC to Satoshis, which allows to get rid of some imports (
COINandDecimal) and a comment for thetest_desc_size_limitssubtest is fixed (s/25KvB/21KvB/).