refactor: test: replace inv type magic numbers by constants #18764
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
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Apr 25, 2020
|
Concept ACK |
16dc435 to
e5d107a
Compare
|
Rebased. |
|
Code review ACK. |
|
e5d107a to
adb0cc4
Compare
Oops, that apparently happened in the course of rebasing, resolving a merge conflict. Fixed. |
glozow
reviewed
May 6, 2020
adb0cc4 to
4a614ff
Compare
|
Rebased on master and added changes suggested by gzhao408 (explicitely importing from |
|
ACK |
11 tasks
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 20, 2020
…y constants 4a614ff test: explicit imports from test_framework.messages in p2p_invalid_messages.py (Sebastian Falbesoner) b35e1d2 test: add inventory type constant MSG_CMPCT_BLOCK (Sebastian Falbesoner) eeaaa58 test: replace inv type magic numbers by constants (Sebastian Falbesoner) Pull request description: Many functional tests still use magic numbers for inventory types, either passed to the `CInv` constructor or for comparing the `type` member of `CInv`. This PR replaces all of those by constants in the module `test_framework.messages` that have been introduced in commit c32cf9f: `MSG_TX` (1) or `MSG_BLOCK` (2). It also introduces a new constant `MSG_CMPCT_BLOCK` (naming as in `src/protocol.h`) and uses it to replace the remaining magic numbers. The occurences of the magic numbers were identified through `grep`ing for `CInv(` and `type ==`. The idea was first to create a scripted-diff, but since also adding missing `import`s is needed, this would be non-trivial. Besides, also some unneeded comments like `# 2 == "Block"` could be removed. ACKs for top commit: gzhao408: ACK [`4a614ff`](bitcoin@4a614ff) Tree-SHA512: 4ba4fdef9f3eef7fd5ac72cb03ca3524863d1ae292161c550424a4c1047283fa2d2e7e03017d1fbae3652b3cb14f08b8d4b368403f3f209993aef3f2e2b22784
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Feb 3, 2021
Summary: This is a backport of Core [[bitcoin/bitcoin#18764 | PR18764]] [1/3] bitcoin/bitcoin@eeaaa58 Test Plan: ninja check-functional Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D9141
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Feb 3, 2021
Summary: Bitcoin ABC already had such a constant, so this just renames it to use the same as Core and uses it in p2p_compactblocks.py This is a backport of Core [[bitcoin/bitcoin#18764 | PR18764]] [2/3] bitcoin/bitcoin@b35e1d2 Depends on D9141 Test Plan: ninja check-functional Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D9142
deadalnix
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Feb 3, 2021
…ssages.py Summary: This is a backport of Core [[bitcoin/bitcoin#18764 | PR18764]] [3/3] bitcoin/bitcoin@4a614ff Depends on D9142 Test Plan: test/functional/test_runner.py p2p_invalid_messages Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D9143
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Many functional tests still use magic numbers for inventory types, either passed to the
CInvconstructor or for comparing thetypemember ofCInv. This PR replaces all of those by constants in the moduletest_framework.messagesthat have been introduced in commit c32cf9f:MSG_TX(1) orMSG_BLOCK(2).It also introduces a new constant
MSG_CMPCT_BLOCK(naming as insrc/protocol.h) and uses it to replace the remaining magic numbers.The occurences of the magic numbers were identified through
greping forCInv(andtype ==. The idea was first to create a scripted-diff, but since also adding missingimports is needed, this would be non-trivial. Besides, also some unneeded comments like# 2 == "Block"could be removed.