Skip to content

Tools: Fix run_filter_test.py to not confuse pytest discovery#30443

Closed
cclauss wants to merge 1 commit intoArduPilot:masterfrom
cclauss:tools-run_filter_test.py-confuses-pytest-discovery
Closed

Tools: Fix run_filter_test.py to not confuse pytest discovery#30443
cclauss wants to merge 1 commit intoArduPilot:masterfrom
cclauss:tools-run_filter_test.py-confuses-pytest-discovery

Conversation

@cclauss
Copy link
Contributor

@cclauss cclauss commented Jun 23, 2025

Tools/FilterTestTool/run_filter_test.py has a filename that will make Pytest Discovery think that it should be tested.

This script also has code at global scope that will fail if certain command line arguments are not provided.

The proposed change is to put that code under if __name__ == "__main__": so it will not be run at test time.

Search for run_filter_test:

How was this tested?

  1. pytest Tools/FilterTestTool/run_filter_test.py
  2. pytest Tools

Both of the above find no tests instead of crashing with a confusing error.

`Tools/FilterTestTool/run_filter_test.py` has a filename that will make Pytest Discovery think that it should be tested.
* https://docs.pytest.org/en/stable/explanation/goodpractices.html#conventions-for-python-test-discovery

This script also has code ___at global scope___ that will fail if certain command line arguments are not provided.

The proposed change is to put that code under `if __name__ == "__main__":` so it will not be run at test time.
* https://docs.python.org/3/library/__main__.html

Search for `run_filter_test`:
* https://github.com/search?q=repo%3AArduPilot%2Fardupilot%20run_filter_test&type=code

How was this tested?
1. `pytest Tools/scripts/build_tests/test_ccache.py`
2. `pytest Tools`

Both of the above find no tests instead of crashing with a confusing error.
@cclauss cclauss force-pushed the tools-run_filter_test.py-confuses-pytest-discovery branch from a00a462 to 406fd43 Compare June 23, 2025 18:49
@cclauss cclauss changed the title Tools: Fix run_filter_test.py to not confuse pytest discovery Tools: Fix run_filter_test.py to not confuse pytest discovery Jun 23, 2025
@tpwrules
Copy link
Contributor

This might be another candidate for aging out. We need to test a log through it to make sure it's not broken but there is for sure some old stuff in it... I agree with the making it python3-specific.

@cclauss
Copy link
Contributor Author

cclauss commented Jun 28, 2025

Closing because even with these changes, .github/workflows/test_unit_tests.yml does not use: setup-python, so the script fails because tkinter is not installed.

@cclauss cclauss closed this Jun 28, 2025
@tpwrules
Copy link
Contributor

Why don't we put pytest tests in an allowlisted folder?

@cclauss cclauss deleted the tools-run_filter_test.py-confuses-pytest-discovery branch June 28, 2025 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants