Functional tests: stabilizing results and a few test fixes. #63
Conversation
Sync with bitcoin test framework, stabilize network connection by waiting for the outgoing node before test starts. Add disable_autoconnect to disable ThreadOpenConnections. Add block generator functions as in the bitcoin test framework. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
asyncio.WindowsSelectorEventLoopPolicy for Windows and send additional msg during p2p connection. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
Sync with bitcoinf implementation for the test init and config. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
BGL has segwit built-in, therefore skip pre-segwit test. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
The test failed due to missing config of the base class, initialize the base class config. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
The test failed due to using bitcoin address list, correct this by using BGL addresses. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
The test failed due to expecting the initial BGL balance to 1250, insert correct inital 5000 BGL balance. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
Fix starting balance to 5000 BGL. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
Fix the test by ensuring that ThreadOpenConnections::start time is properly set. Manually merged from https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_config_args.py@fac23c2 Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
The feature_dersig test is not applicable to BGL, it verifies BIP66 soft fork before and after block 1251. BGL has this built in from block 0: https://github.com/BitgesellOfficial/bitgesell/blob/d3b67375c6931b580aad05afa44b0ae7bae9fcaf/src/chainparams.cpp#L75. Ref: https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021
|
Hi. I noticed some fixes start to overlap with #49, to avoid double work I opened three additional issues, two look like a potential bug, one is a test issue, and I am moving the PR from WIP to 'Ready for review state'. |
gitcoindev
changed the title
|
Hi @janus I am not sure if you are the maintainer of the BGL codebase, I have seen your recent commits to master. |
Thanks. Give me few days. |
Thank you, take your time on this. |
| pass | ||
|
|
||
| def generate(self, generator, *args, sync_fun=None, **kwargs): | ||
| blocks = generator.generate(*args, invalid_call=False, **kwargs) |
There was a problem hiding this comment.
You wrote a lot of new functions. That's a bit unusual.
|
@gitcoindev It would be great if you could add more commits. |
Hi @janus thank you for the feedback, I will split the change that you mentioned into several commits. |
|
Hi @janus I am back, was struck with world's favorite virus, still not perfect but managed to survive. Anyways I like to finish what I started, I had a look at your sync branches, and will likely try to fix functional tests in small batches directing your latest sync branch, perhaps 2-3 tests at a time so that you can review it easily. I hope I can help a bit and use this as a brain puzzle exercise to get me back on track. I am not sure perhaps I will close this PR and move on as it is out of sync now. |
Great news. I wish you the very best. |
|
I worked according to BGL PR bounty hunt but didn't receive any replies as
of yet. Please review my work also.
…On Tue, 21 Dec, 2021, 11:00 am Emeka, ***@***.***> wrote:
Hi @janus <https://github.com/janus> I am back, was struck with world's
favorite virus, still not perfect but managed to survive. Anyways I like to
finish what I started, I had a look at your sync branches, and will likely
try to fix functional tests in small batches directing your latest sync
branch, perhaps 2-3 tests at a time so that you can review it easily. I
hope I can help a bit and use this as a brain puzzle exercise to get me
back on track. I am not sure perhaps I will close this PR and move on as it
is out of sync now.
Great news. I wish you the very best.
—
Reply to this email directly, view it on GitHub
<#63 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AUULJTMBLHX4RC5RK5A75UTUSAGI3ANCNFSM5IESHYWQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Description
Hi, I noticed the Bitgesell contribution program at https://gitcoin.co/issue/bitgesellofficial/bitgesell/62/100027021 also referenced at #62 and decided to help a bit on this.
Notes
Currently after the changes, the test complete in a reasonable time and that the results are reproducible.
Approach taken: I roughly compared latest bitcoin and Bitgesell forks, then had a deeper dive into the test framework and compared against the btc, then ran the BTC and BGL full test suite a few times and started identifying the root cause of the BGL failures.
I already found and fixed a couple of issues with the functional tests. The major fix was stabilizing the RPC connection failures by waiting for the outgoing node to initialize, apart from that I saw a few tests that used BTC addresses and a few pre-segwit that could be partially or fully disabled.
The remaining 40 functional tests I will investigate and try to fix one by one, with the goal to reducing failures close to zero.