-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: functional: Fix mempool-* functional tests. #125
test: functional: Fix mempool-* functional tests. #125
Conversation
* Integrate fixes from master to bitcoinsync2022 branch * Fix new tests adapting from Bitcoin to BGL
Hey @gitcoindev, |
Hi @naftalimurgor thank you!. The 2022 branch does not build out of the box, seems due to some integration hiccup, build system complains about QT and cpp tests Makefiles and this will need to be repaired in the future. The exact error message is already shown in the
Since I am working on functional tests, I do not need QT nor cpp unit tests and I was able to copy missing parts from my
I had to use --with-incompatible-bdb as well (BDB from contrib/install_sb4.sh works perfectly fine on my master branch and I used it for wallet tests). I hope that helps and will enable you to build as well. |
Thanks @gitcoindev |
I am on discord @korrrba , but mostly offline, checking it just from time to time. |
Awesome, I'll shoot you a message there |
The above is not an issue but leftover from legacy code. Bitcoin added some lines to check and link those Makefile during autogen.sh run. However, we have not updated .gitignore to properly pick those files.
I would go through your code (review). |
Got the following: TEST | STATUS | DURATION mempool_accept.py | ✓ Passed | 8 s ALL | ✖ Failed | 164 s (accumulated) I used your branch, * bugfix/bitcoinsync2022-fix-mempool-functional-tests b598dc7 test: functional: Fix mempool-* functional tests. Did you flag updatefromblock.py to be skipped? If yes, it looks like you didn't include it to your commit. I am going to re-run your branch. |
Hi @janus let me check again. I have worked on an older laptop before on Ubuntu 18.04. Now will try with Ubuntu 20.04, both clang and gcc. I did not mark it as skipped, but if it is skipped will check why. |
Hi @janus just a follow up on this, the test was not enabled due to bdb not installed. I installed bdb and it fails, but there is indeed something wrong on bitcoin2022 branch with the memory pool. The test case prepares 100 transactions and mines them in batches of 25 using
I need some more time to find a root cause and fix this. |
Thanks for the update. I will also check it out. |
The error we are witnessing may not have much to do with the branch, bitcoinsync2022. Bitgesell has less block weight than Bitcoin.
However, the below passed confirming the above statement
I am going to do thorough checks later. |
Hi @janus ah this makes sense! I tried to go back in time and use bisection to get a commit that works but struggled with build errors on most commits, the ones I found still had an error. But I also made the test case to pass by asking generate to mine more blocks with this patch:
with the result
perhaps we could use this one or your approach after you finish thorough checks. |
Hi @janus you were absolutely right about 400 000 block limit for BGL, I added a new list in the test case which calculated transaction weight and logged the size:
which gave:
In this case 4 transactions were over 400 000 limit, and were not mined. I will provide a fix for this test and push to this branch now. |
The test case failed when mempool size with generated transactions was larger than BGL max block size (400000). Fix the test case by generating an exact number of blocks to empty the mempool.
I pushed a new commit, all functional mempool test cases are passing now
|
Alright. |
Goal of this pull request is to fix
mempool_*
functional tests in bitcoinsync2022 branch.Notes