ENH: Fail Python tests on warnings during runtime #866
Conversation
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
Looks good but can you document in the commit log the reason for adding all these options? Can you also add a commit introducing a warning to check that it works? I would remove the commit before merging.
axel-grc
changed the title
|
I removed rtkPolynomialGainCorrectionImageFilter from WRAPPER_SUBMODULE_ORDER, this should return a warning and the test should fail. |
| pip install pytest matplotlib | ||
| pytest $GITHUB_WORKSPACE/test/*.py -vv -W "ignore:builtin type swig" | ||
| pytest $GITHUB_WORKSPACE/test/*.py -vv -s -W error -W "ignore:builtin type swig" 2> /tmp/stderr.log |
There was a problem hiding this comment.
What's the purpose of -W "ignore:builtin type swig"?
I'd suggest to use mktemp in place of /tmp/stderr.log to avoid problems with, e.g., two process writing on the same file.
There was a problem hiding this comment.
Just to ignore some known issues in SWIG (tracked in swig/swig#2881). The warnings appear because SWIG is not setting the module attribute on its builtin types.
This should be fixed when ITK use SWIG 4.4.
I changed /tmp/stderr.log for mktemp
axel-grc
force-pushed
the
PythonTest
branch
2 times, most recently
from
5ffdd0d to
0471c2a
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
That does not seem to work as the tests pass and rtkPolynomialGainCorrectionImageFilter jhas been removed from WRAPPER_SUBMODULE_ORDER?
axel-grc
force-pushed
the
PythonTest
branch
5 times, most recently
from
d4a4e53 to
be99727
Compare
- Redirect stderr to mktemp to capture C++ warnings from ITK modules - Check stderr log for "Warning" messages and fail the build if found This ensures the CI catches C++ warnings like unknown parameter from SWIG.
2 participants
This ensures the CI catches both Python and C++ warnings like unknown parameter from SWIG.