Fix/merge test improvements#95
Conversation
Update plausible redirect
Update logo in main version
Major update of the backend to help strenghten the different aspects of the deployment: - Incremental update of the db - API docs - API CORS restriction optional -
Release - fixes to downloader and nerc platform list
… remove gzip_types * since that is dangerous, and avoiding gzipping the frontend until verified
Hotfix: prevent JS bundle truncation on explore.cioos.ca (nginx gzip / sendfile)
Improve deployment
…test. Use this as a baseline for further development
… how it went with resulting harvest reports, additionally, adding in documents too
…nd could possible remove, will keep it for now and review on git PR
Resolves conflicts in .env.production, docker-compose.production.yaml, nginx/nginx.conf, and uv.lock after merging OBIS integration and harvest-dashboard changes from master.
Remove web-api/__tests__/, erddapServers.js, obisNodes.js, scientificNames.js and restore modified web-api files to master state. All web-api work is now tracked on feat/frontend-tests.
|
Okay, I removed all of the web-api changes into a separate branch this one should be able to go through now unless we have some more comments or concerns. @RichardKelly84 @JessyBarrette |
|
Whoops, I removed some changes in the web-api, fixing.... |
vmehraOCN
left a comment
vmehraOCN
left a comment
There was a problem hiding this comment.
Okay, review done on my side.
|
@vmehraOCN could you update the title and description of the PR to reflect the actual changes suggested. The changes
This will help reducing which tests are run when. |
Each package now owns its tests alongside its code:
harvester/tests/
conftest.py shared ERDDAP fixtures and mock data
pyproject.toml depends on harvester + db-loader (for integration)
unit/ 9 harvester unit tests + csv_generation + obis_geo_filter
integration/ full pipeline test (harvest → CSV → db-load)
db-loader/tests/
conftest.py standalone DataFrame fixtures
pyproject.toml depends on db-loader + harvester (runtime dep)
unit/ test_db_loader.py (13 tests)
Results: 141 harvester tests + 13 db-loader tests = 154 total (unchanged).
Remove top-level tests/ directory.
Replace ASCII art uml_diagram.txt with uml_diagram.md containing two Mermaid diagrams (flowchart + classDiagram) and a component table. Also updates stale references: harvest_erddap.py → erddap_harvester.py, adds OBISHarvester, ObisGeoFilter, HarvestResult, BaseHarvester, and harvest-dashboard service.
|
Added in the changes requested @JessyBarrette. |
|
Note, I left the integration test I made within harvester since its is an integration test for the harvester. |
JessyBarrette
left a comment
JessyBarrette
left a comment
There was a problem hiding this comment.
db-loader
The added pyproject.toml in db-loader/tests/pyproject.toml needs to me integrated within the main pyproject of db-loader as a dev depedancies.
harvester
We need to migrate the pytest dependancies to dev. We dont need them in production.
Ideally we would also need to run those during the CI/CD. The question is when and how.
There was a problem hiding this comment.
Not sure if those changes are coming from you or those are in development but not already in feat/harvest-dashboard, but they could be seperated if not already in development. I believe they are though
There was a problem hiding this comment.
I think this is addressed too unless I haven't understood what you mean here.
- Rename test_downloader.yaml to test-unit-tests.yaml - Consolidate into single job that detects changed components - Uses dorny/paths-filter to identify which directories changed - Installs all dependencies once from root pyproject.toml - Runs pytest conditionally for each modified component (harvester, db-loader, downloader) - Adds pytest dev dependencies to root pyproject.toml for shared test tooling Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
I added some of the depandcies to the pyprojects and reviewer the original test_downloader.yaml action to run the different unit tests. Now named test-unit-tests.yaml, it will run the unit tests within each modules if changes are made within them. |
…r tests. Updated uv.lock files to reflect the new dependancies. Added untracked files that are relevant to the testing and development process.
… import Remove unused cde_harvester import from db-loader/__main__.py, add prefect as an explicit runtime dependency, merge pytest config into db-loader/pyproject.toml, and delete the now-redundant db-loader/tests/pyproject.toml and uv.lock.
…toml Add pytest-cov and pytest config (testpaths, addopts) to harvester/pyproject.toml and delete the now-redundant harvester/tests/pyproject.toml and uv.lock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
That sounds pretty good to me! |
|
@RichardKelly84 @JessyBarrette I think I've addressed all current concerns on this PR. Please let me know if there is anything else to review. |
RichardKelly84
left a comment
RichardKelly84
left a comment
There was a problem hiding this comment.
Most of the CSV_generation can probably be removed or updated, some other small comments.
| # Tests: ASCII unescaping | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| class TestUnescapeAscii: |
There was a problem hiding this comment.
GAP: no test here for when there are actual escapes, for example: assert unescape_ascii(r"Caf\u00e9") == "Café"
| # --------------------------------------------------------------------------- | ||
|
|
||
| class TestGetProfilesHappyPath: | ||
| def test_returns_dataframe(self, single_station_dataset): |
There was a problem hiding this comment.
can probably remove this one.. If it doesn't return a df, other tests would fail anyways, I.E. test_result_is_not_empty.
There was a problem hiding this comment.
Checking for type is likely okay here. Its a bit redundant but it won't take much time and does ensure we are working with the correct datatype.
…via code from Dataset, Profiles, and Skipped datasets.
4 participants
Overview
This branch builds a comprehensive backend test suite on top of the OBIS integration work merged from
feat/harvest-dashboard. It resolves several integration regressions, runtime errors, and broken test paths that surfaced during the branch merge.Key Changes
➕ New Additions
tests/): Added a fullpytestsuite from scratch including 113 unit tests across 10 modules (covering ERDDAP, CKAN, and compliance tracking), an end-to-end chained integration test with mocked I/O, and standalone dependency wiring.🛠️ Fixes Implemented
erddap_harvester.py): Fixed a critical production risk wheredataset_loggerwas referenced in an HTTP error block before assignment. It is now safely pre-initialized.MissingContextErrorin unit tests by implementing atry/exceptfallback to standard Pythonlogging.getLogger()when run outside active Prefect flows.main.fn(...)and patched loggers to bypass local Prefect API connection attempts.harvest_erddap.pytoerddap_harvester.pyrename, and corrected a mismatched exception assertion (ResponseTooLargeError) in the ERDDAP client tests.Testing Checklist
cd tests && uv run pytestuv run pytest tests/integration/