sync_all() is used through the test to wether check for block or tx
sync, making it unclear what the test is actually checking.
Using wait_for_tip() and wait_for_tx() makes it slightly clearer.
Depends on D2629
Details
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project
./test/functional/test_runner.py rpc_listtransactions
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- rpc_listtransaction_remove_sync_all
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5108 Build 8279: Bitcoin ABC Buildbot (legacy) Build 8278: arc lint + arc unit
Event Timeline
Comment Actions
Why is that better at all to explicit a bunch of things that are not at all related to listtransactions, which presumably is what is checked ?
Comment Actions
The main intent is to make more clear what step relies on what data to propagate. sync_all() always do both wait_for_tip() and wait_for_tx() every time, I found that separating these makes the test more easy to follow. As a bonus it eventually avoids having the test to pass due to side effects of sync_all() syncing both blocks and mempools when only one is expected, and should makes the test marginally faster.
Comment Actions
The API surface this overs now has increase quite a bit - so the test will be more fragile, it's unclear what side effect of sync_all would cause this test to pass erroneously as this test is not testing how node synchronize, and the fact that marginally faster is not measured makes it a dubious argument at best.
I do not see a good reason to take over this test at this stage rather than simply backporting.