Skip to content

Enable integration tests for all services (fixes #4)#34

Open
marcosfelt wants to merge 3 commits intomainfrom
fix_ci_1
Open

Enable integration tests for all services (fixes #4)#34
marcosfelt wants to merge 3 commits intomainfrom
fix_ci_1

Conversation

@marcosfelt
Copy link
Contributor

This PR addresses issue #4 by enabling integration tests for all services.

Changes

Test Infrastructure

  • ✅ Restored MoleculeResolverPatched class that mocks _resilient_request to cache API responses
  • ✅ Created responses.json file for storing cached test responses

Services with Tests Enabled

  • ✅ OPSIN
  • ✅ OPSIN batchmode
  • ✅ PubChem
  • ✅ CompTox
  • ✅ CTS
  • ✅ CAS Registry
  • CIR (this was the missing one from the issue!)
  • ✅ NIST

Services Kept Commented

  • ⏸️ Chemeo (requires API key)
  • ⏸️ ChEBI (benchmark data lacks chebi_id field)

Fixes

  • Fixed method name from get_molecule_from_CAS_registryx to get_molecule_from_CAS_registry
  • Fixed test_opsin_batchmode to properly access r[0].SMILES instead of r.SMILES

Testing

Tests will run automatically in GitHub Actions CI pipeline. The tests use cached responses to avoid hitting external APIs during CI runs.

Closes #4

This commit addresses issue #4 by:
- Restoring the MoleculeResolverPatched class to mock _resilient_request
- Uncommenting tests for all services: OPSIN, PubChem, CompTox, CTS, CAS, CIR, NIST
- Enabling the OPSIN batchmode test
- Creating responses.json file for caching test responses
- Keeping Chemeo test commented (requires API key)
- Keeping ChEBI test commented (benchmark data lacks chebi_id field)
- Fixing method name from get_molecule_from_CAS_registryx to get_molecule_from_CAS_registry
- Fixing test_opsin_batchmode to properly access result data

Tests now run for all major services as requested in the issue.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@marcosfelt marcosfelt requested a review from simonmb February 16, 2026 14:23
@marcosfelt
Copy link
Contributor Author

@simonmb A quick attempt using claude code

Added missing offline_status_codes parameter and updated type hints
to match the actual MoleculeResolver._resilient_request signature.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@simonmb
Copy link
Contributor

simonmb commented Feb 16, 2026

@marcosfelt, thanks for this! For ChEBI, do we actually need the IDs? We could also search by name like with the others? Or am I missing something?

Changed bare 'raise' to 'raise ValueError' with descriptive message
when property count is not exactly 1. This fixes RuntimeError in tests.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@marcosfelt
Copy link
Contributor Author

@simonmb I'm guessing you're right. I haven't looked at CHEBI in over two years.

On another note, these tests take a long time (30 minutes) - should we only run them on a schedule?

@simonmb
Copy link
Contributor

simonmb commented Feb 17, 2026

Yeah, I saw they took this long. But why though? We have about 100 entries, or am I wrong? Using the online version actually getting the results from the services should be faster than what it's taking now by getting the results from a mock. I suspect something is off. Did you run this locally?

@simonmb
Copy link
Contributor

simonmb commented Feb 21, 2026

We definitely need to add a call to search ethanol on every service online as well. I just noticed by chance that the CAS registry introduced an obligatory api key to their requests. Will work on that later. But having a test on online accessibility for one pretty well known component makes sense.

@simonmb
Copy link
Contributor

simonmb commented Feb 22, 2026

I just realized 2 services weren't working. (I just fixed them) So we should add online searches to the tests, There were a lot of changes in the past 6 months on the services side...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement basic integration tests and continuous integration

2 participants