DDOS-5352: Test cleanup - #523
Conversation
There was a problem hiding this comment.
Pull request overview
This PR primarily refactors tests to use the shared client pytest fixture, while extending system-prep result handling to support an optional solute-only PDB output and bumping the configured system-prep function version.
Changes:
- Update many tests to accept a
client: DeepOriginClientfixture instead of instantiating clients inline. - Add
solute_pdb_pathsupport toPreparedSystem(parsing, hydration, and visualization viashow(solute=True)), and propagate it from system-prep outputs / ABFE DTOs. - Bump
TOOL_KEYS_AND_VERSIONS["sysprep"]["function_version"]to0.10.0.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_tools_api.py | Switch tests to the shared client fixture. |
| tests/test_protein.py | Add client fixture parameter across protein tests (plus import). |
| tests/test_projects.py | Switch to client fixture and replace randomized names with fixed strings. |
| tests/test_progress_reports.py | Switch tests to the shared client fixture. |
| tests/test_prepared_system.py | Add coverage for solute_pdb_path parsing and show(solute=True) error handling. |
| tests/test_organizations.py | Switch tests to the shared client fixture. |
| tests/test_ligand_set.py | Add client fixture parameter broadly to ligand set tests. |
| tests/test_ligand.py | Add client fixture parameter broadly to ligand tests; use fixture env in one skip check. |
| tests/test_file_api.py | Switch tests to the shared client fixture. |
| tests/test_executions.py | Switch tests to the shared client fixture and adjust typing ignores. |
| tests/test_entities.py | Switch tests to the shared client fixture. |
| tests/test_deeporigin_projects.py | Switch tests to the shared client fixture and use client.env/client.project_id. |
| tests/test_datasets.py | Switch tests to the shared client fixture. |
| tests/test_clusters.py | Switch tests to the shared client fixture. |
| tests/test_billing_tag_end_to_end.py | Switch to shared client fixture and remove inline construction. |
| tests/test_abfe.py | Switch ABFE tests to accept client fixture and remove inline construction in a few places. |
| src/platform/constants.py | Bump system-prep function version to 0.10.0. |
| src/drug_discovery/system_prep.py | Capture/propagate optional solute_pdb_file_path into PreparedSystem. |
| src/drug_discovery/structures/prepared_system.py | Add solute_pdb_path, extend show() to support solute=True, and add filtering by compute_job_id in from_result. |
| src/drug_discovery/abfe.py | Propagate optional solute_pdb_file_path into PreparedSystem during DTO hydration. |
| .vscode/settings.json | Extend spellchecker dictionary entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| unique_a = "CLI test test_projects_search_name_icontains Alpha Project" | ||
| unique_b = "CLI test test_projects_search_name_icontains Beta Workspace" | ||
|
|
||
| client.projects.create(name=unique_a) | ||
| client.projects.create(name=unique_b) |
There was a problem hiding this comment.
These tests now use fixed project names. Since client.projects.create() always generates a unique slug, this will create a new project on every run; over time search(..., limit=50) can stop returning the just-created rows (and may return unrelated existing projects), making the assertions flaky. Consider restoring a per-run unique suffix (e.g., UUID) or deleting the created projects, and/or assert using the IDs returned from create() instead of relying on substring search within a limited page.
| name = "CLI test test_projects_user_create_upserts_by_exact_name" | ||
| first_id = create(name=name, load=False) | ||
| second_id = create(name=name, load=False) | ||
| assert isinstance(first_id, str) | ||
| assert first_id == second_id |
There was a problem hiding this comment.
deeporigin.projects.create() uses projects.search(name=..., limit=1) under the hood and then filters for an exact name match. With a fixed name that shares a common prefix with other test projects, limit=1 can return a different row and cause create() to create duplicates (breaking the upsert expectation). Use a truly unique name per run (UUID suffix) or pass an explicit client= and search by exact name with a larger limit before creating.
No description provided.