Updates for testing suite #70
Conversation
BradyPlanden
commented
Oct 14, 2025
BradyPlanden
commented
- Adds initial Casadi function based integration tests
- Removes simplistic unit tests
| try: | ||
| import casadi | ||
|
|
||
| CASADI_AVAILABLE = True | ||
| except ImportError: | ||
| CASADI_AVAILABLE = False |
There was a problem hiding this comment.
I would recommend using casadi = pytest.importorskip("casadi") for this. https://docs.pytest.org/en/stable/reference/reference.html#pytest-importorskip
| These tests verify interactions between different components | ||
| and may require more complex setup or be slower to run. | ||
| These tests verify that the C++ pybindings work correctly when called from Python |
There was a problem hiding this comment.
| These tests verify that the C++ pybindings work correctly when called from Python | |
| These tests verify that the C++ bindings work correctly when called from Python |
| import pybammsolvers | ||
|
|
||
| pytestmark = pytest.mark.unit | ||
| assert hasattr(pybammsolvers, "idaklu") |
There was a problem hiding this comment.
| assert hasattr(pybammsolvers, "idaklu") | |
| import pybammsolvers.idaklu |
hasattr will only check if the attribute exists, but not actually import it – which means that we aren't testing whether the extension module is importable unless we use an import statement.
| # JAX integration | ||
| assert hasattr(idaklu_module, "IdakluJax") | ||
| assert hasattr(idaklu_module, "create_idaklu_jax") |
There was a problem hiding this comment.
For these tests, the package is already imported in the conftest.py, so we are checking that the imported package has the corresponding attributes. Unless I'm missing something here?
| # Verify they're callable/instantiable | ||
| assert callable(idaklu_module.solution) | ||
| assert callable(idaklu_module.VectorNdArray) | ||
| assert callable(idaklu_module.VectorSolution) | ||
| assert callable(idaklu_module.create_idaklu_jax) |
There was a problem hiding this comment.
These can be replaced with imports as well
There was a problem hiding this comment.
Same as above, checking that they are callable after import
|
Ah, just noticed that the PR was merged before I could put in my review. @BradyPlanden, if it's okay, could you perhaps incorporate these suggestions in another PR (or add them in #69)? |
3 participants
