Feat: Functional tests.

[jaoleal]

As saw #198 (comment). were only certain that our implementation is properly working if we run the node. This is not ideal because is resource and time consuming, not all devices that can be used to develop rust code can easily run the node.

This issue is to keep track of implementing new functional tests. Such as:

Specific blocks evaluation.
Mass blocks evaluation.
Conection with other nodes.

A good example to follow is the functional tests on Bitcoin Core.

[qlrd]

Hi all,

Talked with @jaoleal that i have some interest in work with functional tests.

Started to read tests code and an idea become in the sense to follow some bitcoin-core's style guidelines.

So, before make a PR (already make a simple change locally as "proof"), i would like to discuss the proposition (not for functional tests itself, but for a strucuture that can improve time for developers of functional tests):

• isolate possible python environments with poetry/venv and its dependencies (so it will be possible to test in py3.9, py3.10, etc.);
• format python code with black;
• lint python code with pylint;
• organization of tasks when the codebase for functional tests increase;
• a manager for tasks above with poethepoet.

This can be applied with a simple addition of a pyproject.toml (maybe in the project's root directory or in the tests directory) and an entry in the README describing the procedures to install and run.

This proposition, on my machine, produced this outputs when comes to formating, linting and tests:

Format

❯ poetry run poe format
Poe => black ./tests
reformatted /home/qlrd/github/Floresta/tests/restart.py
reformatted /home/qlrd/github/Floresta/tests/example_test.py
reformatted /home/qlrd/github/Floresta/tests/run_tests.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/test_framework.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/floresta_rpc.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/mock_rpc.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/electrum_client.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/utreexod.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/key.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/secp256k1.py
reformatted /home/qlrd/github/Floresta/tests/test_framework/bitcoin.py

Lint

❯ poetry run poe lint
Poe => pylint ./tests
************* Module example_test
tests/example_test.py:21:8: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return)
tests/example_test.py:21:8: W0612: Unused variable 'node1' (unused-variable)
tests/example_test.py:4:0: W0611: Unused import time (unused-import)
tests/example_test.py:5:0: W0611: Unused import os (unused-import)
tests/example_test.py:9:0: W0611: Unused MockUtreexod imported from test_framework.mock_rpc (unused-import)
************* Module restart
tests/restart.py:21:0: C0325: Unnecessary parens after 'assert' keyword (superfluous-parens)
tests/restart.py:1:0: C0114: Missing module docstring (missing-module-docstring)
tests/restart.py:8:0: C0115: Missing class docstring (missing-class-docstring)
tests/restart.py:15:8: E1120: No value for argument 'net' in method call (no-value-for-parameter)
tests/restart.py:18:8: E1120: No value for argument 'net' in method call (no-value-for-parameter)
tests/restart.py:1:0: W0611: Unused import subprocess (unused-import)
tests/restart.py:3:0: W0611: Unused import os (unused-import)
************* Module run_tests
tests/run_tests.py:1:0: C0114: Missing module docstring (missing-module-docstring)
tests/run_tests.py:11:0: C0116: Missing function or method docstring (missing-function-docstring)
tests/run_tests.py:22:19: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
tests/run_tests.py:38:17: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
tests/run_tests.py:41:8: W0719: Raising too general exception: Exception (broad-exception-raised)
tests/run_tests.py:22:19: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
tests/run_tests.py:23:15: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
...<A bunch of lint errors>
...
------------------------------------------------------------------
Your code has been rated at 6.75/10 (previous run: 7.13/10, -0.39)

Tests

❯ poetry run poe test
Poe => python tests/run_tests.py
Creating work dir
writing stuff to /tmp/datarun-1734387879.3541138/
Test example_test passed ✔
Test restart passed ✔

[jaoleal]

i would like to discuss the proposition (not for functional tests itself, but for a strucuture that can improve time for developers of functional tests):

Looks good guidelines for initiating our functional tests in python... All the proposed tools to improve python coding environment inside this project can be implemented in our nix-shell too...

GJ!

Feel free to ask me any questions if you decide to contribute as we discussed outside github

[qlrd]

Nice!

I thought in a simple 3 PR plan to start:

• 1st PR: add the proposed pyproject.toml and README changes that will give the structure and steps for functional test developers (see Structure improvement functional tests #309);

• 2nd PR: changes on python code just to format and fix lint errors;

• 3rd PR: a change on functional CI -- on Run functional tests step -- to apply the above PRs on actions;

I think it can be more reviewable that way, or is it better to do the 2nd and 3rd together?

[Davidson-Souza]

Hi @qlrd

So, before make a PR (already make a simple change locally as "proof"), i would like to discuss the proposition (not for functional tests itself, but for a strucuture that can improve time for developers of functional tests):

Sounds very promising! Thanks for working on this!

[jaoleal]

I think it can be more reviewable that way, or is it better to do the 2nd and 3rd together?

Maybe the 1st and the 2nd can be merged since they are reforming the old code... the 2nd is just auto linting right ? no need for a separate PR just for it

[qlrd]

Maybe the 1st and the 2nd can be merged since they are reforming the old code... the 2nd is just auto linting right ? no need for a separate PR just for it

in line with the idea, it seems more succinct. Just to point, the 2nd isn't a auto-linting (i think is possible to do auto-linting, except the docstrings).

I will fix the @Davidson-Souza points before do the linting commit.

[qlrd]

Created some functional tests for floresta-cli (see qlrd@f48ca6a and qlrd@0684896).

Made with some commands that was more-or-less intuitive.

Didn't made with others because didnt understood how to generate some regtest blocks (to be able to test sendrawtransaction, gettransaction, etc...), or specialized ones (getroots for example).

• getblockchaininfo
• getblockhash
• gettxout
• gettransaction
• rescan
• sendrawtransaction
• getblockheader
• loaddescriptor
• getroots
• getblock
• getpeerinfo
• getpeerinfo (expect error when in IDB)
• stop
• addnode (expect error when in IDB)
• add node

[Davidson-Souza]

Didn't made with others because didnt understood how to generate some regtest blocks (to be able to test sendrawtransaction, gettransaction, etc...), or specialized ones (getroots for example).

I've been considering adding a generate rpc to create regtest blocks. I think it would help with this.

[qlrd]

Thanks @Davidson-Souza. This would be great!

This command will allow to advance in several tests.

For now I have been relying on expected errors from a node in an IDB state (Node is in initial block download, wait until it's finished message).

Just updated the list above with the implemented some functional tests in my repo. Once done them, a PR will be done from my side.