Skip to content

Conversation

@moisesPompilio
Copy link
Collaborator

Description and Notes

Pytest, a widely used Python testing framework, has been integrated into the functional test suite. Pytest simplifies test creation, execution, and management, making it easier to scale and maintain the test suite as it grows.

  • Example tests were refactored to follow the pytest standard, providing a consistent and clear structure for future tests.
  • The project's run.sh script was updated to support running both the new pytest-based functional tests and the existing, non-migrated tests.
  • Pytest was configured with multi-threading support (defaulting to 4 workers) to improve test run efficiency and speed.
  • Test execution can now be controlled with flags: use --pytest in run.sh to run only the pytest-based tests, or --test-runner to execute the legacy test runner.
  • This PR supersedes PR Refact test runner #671, which previously aimed to introduce pytest-based functional tests. The current implementation provides an updated and more focused approach to integrating pytest, in collaboration with @j-moreno-c-r.

Note: This PR depends on the changes from PR #731 to function properly.

How to verify the changes you have done?

  • Run the functional tests via run.sh and confirm that both the pytest-based and legacy functional tests are executed.
  • Use the --pytest flag with run.sh to run only pytest-based tests and verify expected output and results.
  • Use the --test-runner flag to run only legacy functional tests.
  • Check that multiprocessing (4 workers by default) functions as intended during pytest test runs.
  • Confirm that refactored example tests follow the pytest pattern and are correctly discovered and executed by pytest.

@joaozinhom
Copy link
Contributor

joaozinhom commented Dec 20, 2025

image

LGTM This won't work on NixOS because for somehow a change in the name of the binary from bitcoind to bitcoin in the flake was merged, and it won't be recognized by the test_framework or even the pytest tests, this failure on ping needs to be reviewed, but locally everything its fine after i locally fix the flake adding a "d" in this printed line.

@moisesPompilio
Copy link
Collaborator Author

LGTM This won't work on NixOS because for somehow a change in the name of the binary from bitcoind to bitcoin in the flake was merged, and it won't be recognized by the test_framework or even the pytest tests, this failure on ping needs to be reviewed, but locally everything its fine after i locally fix the flake adding a "d" in this printed line.

Thanks for letting me know about this detail, I already made the change.

@jaoleal
Copy link
Collaborator

jaoleal commented Dec 20, 2025

image LGTM This won't work on NixOS because for somehow a change in the name of the binary from bitcoind to bitcoin in the flake was merged, and it won't be recognized by the test_framework or even the pytest tests, this failure on ping needs to be reviewed, but locally everything its fine after i locally fix the flake adding a "d" in this printed line.

It works, this devshell is just a more nix native approach for doing exactly the same thing than prepare.sh.

It was necessary when I tried to do a check with our tests but our tests depends on things that nix doesnt want to provide in checks environment.

Now it just lost its purpose, with a nix-shell -p uv python312 go cmake clang boost and run the tests using the just recipes youll accomplish the same that this nix devshell is actually being usefull for. Probably will be better even not using it because it can trigger some unecessary and expensive recompiles.

I actually want to get rid of it lol, i hate it.

@jaoleal
Copy link
Collaborator

jaoleal commented Dec 20, 2025

@moisesPompilio Do you think that we could remove this nix shell on #716 ?

@moisesPompilio
Copy link
Collaborator Author

@moisesPompilio Do you think that we could remove this nix shell on #716 ?

Yes, I did that in this commit: 7d2d02f

@moisesPompilio
Copy link
Collaborator Author

I did the rebase and added the logger part to be used in pytest that was missing, so we have the logger files like in the test without pytest. However when a pytest test fails it already displays the failure in the terminal which saves time searching the log file. Besides that, now we have logger levels. In this case the default level is debug.

@luisschwab
Copy link
Member

Needs rebase

@moisesPompilio
Copy link
Collaborator Author

32fb3e9 Rebase done

Davidson-Souza added a commit that referenced this pull request Feb 10, 2026
42c63a9 fix: cleanup test directories before running tests - Keep unconditional /data cleanup at start - Make /logs cleanup conditional on --preserve-data-dir flag (Eduardo Augusto)

Pull request description:

  Related to #760 (mitigation)

  Keep unconditional /data cleanup at start
  Make /logs cleanup conditional on --preserve-data-dir flag
  Prevents reusing old log state between test runs
  Description and Notes
  Currently, the cleanup of /logs directory happens at the END of test execution and only if tests pass. This causes:

  Failed tests leave corrupted log state for next run
  Tests may fail when run multiple times without manual cleanup
  This PR provides a temporary mitigation by making /logs cleanup conditional on the --preserve-data-dir flag, while keeping /data unconditionally cleaned at start.

  Note: This is a partial fix. Complete resolution will come in a follow-up PR after #742 is merged, where cleanup logic will be migrated to the Python runner for better control over test data management.

  How to verify the changes you have done?
  # Test 1: Run tests multiple times
  ./tests/prepare.sh
  ./tests/run.sh
  ./tests/run.sh  # Should work without manual cleanup

  # Test 2: Preserve logs for debugging
  ./tests/run.sh --preserve-data-dir
  ls -la $FLORESTA_TEMP_DIR/logs  # Logs preserved
  Next Steps
  After #742 is merged, I plan to submit a follow-up PR to:

  Migrate cleanup logic to the Python test runner
  Provide more granular control over test data management
  Fully resolve #760
  Contributor Checklist
   I've followed the [contribution guidelines](https://github.com/vinteumorg/Floresta/blob/master/CONTRIBUTING.md)
   I've verified one of the following:
   Ran just pcc (skipped - changes are bash-only, validated manually)
   Ran just lint-features '-- -D warnings' && cargo test --release (not applicable)
   Confirmed CI passed on my fork (will run after PR creation)
   I've linked any related issue(s) in the sections above
  Manual validation performed:

  ✅ Bash syntax check (bash -n)
  ✅ Logic tested with simulations
  ✅ --preserve-data-dir flag tested
  ✅ Consecutive test runs verified

ACKs for top commit:
  moisesPompilio:
    ACK 42c63a9
  jaoleal:
    ACK 42c63a9

Tree-SHA512: b3c9f62d4ce612e7231748aeae7352cb89c725243e69251879291af9ca1083a5ca2774405034394df39c407bd0005d87950c79b76740f4442be5e842006bb458
@Davidson-Souza
Copy link
Member

Needs rebase again

moisesPompilio and others added 3 commits February 10, 2026 20:21
Introduce pytest as the new testing framework to simplify and enhance the way
tests are written and executed. Pytest facilitates the creation of reusable
functions and fixtures, making it easier to manage and instantiate nodes
dynamically. It also provides built-in assertion functions and a robust
test runner.

co-authored-by: joaozinhom <joaozinhom@users.noreply.github.com>
Introduce multithread support for pytest in the functional tests, enabling tests
to run in parallel with a default of 4 workers.

To ensure proper isolation and avoid conflicts between tests running simultaneously,
the `/data` directory for nodes is now generated dynamically based on the name of
the test being executed. This adjustment leverages the `request.node.name` attribute
to create unique data directories for each test.

co-authored-by: joaozinhom <joaozinhom@users.noreply.github.com>
…test_runner.py

Migrated the Electrum example test to use pytest, simplifying the test structure.
Removed the requirement for all tests to be listed in test_runner.py, allowing
tests to be run independently with pytest.

co-authored-by: joaozinhom <joaozinhom@users.noreply.github.com>
@moisesPompilio
Copy link
Collaborator Author

165b340 Rebase done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request functional tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants