Add unit tests for base helper modules#797
Add unit tests for base helper modules#797cybermaggedon merged 1 commit intotrustgraph-ai:masterfrom
Conversation
Contributor License Agreement ✅All contributors have signed the CLA. Thank you! |
|
I have read the CLA Document and I hereby sign the CLA |
cybermaggedon
left a comment
cybermaggedon
left a comment
There was a problem hiding this comment.
Thanks for the PR. Can you remove the customer loader, and pytest.importorskip("pytest") and this is good to merge.
| from tests.unit.test_base._module_loader import load_base_module | ||
|
|
||
|
|
||
| pytest.importorskip("pytest") |
There was a problem hiding this comment.
Suggest deleting this. This module runs under pytest, so if this code is executing, pytest is running.
| from pathlib import Path | ||
|
|
||
|
|
||
| BASE_DIR = Path(__file__).resolve().parents[3] / "trustgraph-base" / "trustgraph" / "base" |
There was a problem hiding this comment.
The custom loader is unnecessary, and extends risks of executing unwanted code in the CI pipeline. We want to be very 100% that the tests are testing the right code, so the procedure is that you install the packages and then execute the tests.
e.g.
python3 -m venv env
. env/bin/activate
# This is the subset of packages the tests extend over
pip install ./trustgraph-{base,cli,flow,vertexai,bedrock,unstructured}
And then you can run the tests...
pytest tests/unit
pytest tests/integration -m 'not slow'
pytest tests/contract
There was a problem hiding this comment.
Btw, there's something faulty about pip install -e for these packages, if you're used to using that. It doesn't work because they share a 'trustgraph' namespace, which pip install -e doesn't know how to deal yet.
zeel2104
force-pushed
the
test/base-module-coverage-792
branch
from
d8c469e to
4e63dbd
Compare
|
@cybermaggedon python -m pytest -o addopts="" tests/unit/test_base/test_metrics.py tests/unit/test_base/test_logging.py tests/unit/test_base/test_flow_base_modules.py |
cybermaggedon
left a comment
cybermaggedon
left a comment
There was a problem hiding this comment.
Both issues fixed. Custom module loader is gone — all three files use normal imports. pytest.importorskip("pytest") is gone. The parameter_spec.py bug fix (adding self to start/stop) is still included. Good to merge.
|
Tests are failing, but that's because we're tracking some new changes on master from the v2.2 / v2.3 development. Merging this, because merging onto a release branch will likely fix most of this. |
- add unit tests for base metrics, logging, spec, parameter_spec, and flow modules - add a lightweight test-only module loader so these tests can run without optional runtime dependencies - fix Parameter.start/stop to accept self
|
Thanks for the additional test coverage, much appreciated! ❤️ |
2 participants
Summary
Testing
Closes #792