Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces structured tests for various model endpoints and updates to test configurations. Key changes include:
- Adding new constants and test cases in tests/variables_test.py for additional models.
- Updating test files to adjust the working directory for rootutils setup.
- Introducing new API test files covering both successful and failure scenarios for retrosynthesis endpoints, along with updated GitHub workflows for LLM and API tests.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/variables_test.py | Added constants for new models and API configuration variables. |
| tests/llm-tests/test_llm_parsers.py, test_llm.py, test_adv_prompt.py | Updated root directory setup for tests. |
| tests/api-tests/* | New test files for various endpoints (retrosynthesis, health, call failures). |
| .github/workflows/llm-tests.yml, .github/workflows/api-tests.yml | Added CI workflows for running the new tests. |
Comments suppressed due to low confidence (1)
tests/variables_test.py:10
- The constant CLAUDE_ADV_MODEL is used in several test files but is not defined in this file. Please add the missing definition.
DEEPSEEK_FIREWORKS_MODEL = "fireworks_ai/accounts/fireworks/models/deepseek-r1:adv"
There was a problem hiding this comment.
Pull Request Overview
This PR introduces new tests for the API and LLM modules, additional CI configurations for running these tests, and general cleanup of the code base.
- Added new tests for retrosynthesis endpoints with various model configurations
- Updated CI workflows for LLM and API tests
- Made minor adjustments to test utility configurations
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/variables_test.py | Added constants for models and updated BASE_URL and endpoints |
| tests/llm-tests/*.py | Updated root directory setup for consistency |
| tests/api-tests/*.py | Added API tests for retrosynthesis, rerun, health, and failure cases |
| .github/workflows/llm-tests.yml, api-tests.yml | New CI workflows to run LLM and API tests on push/PRs |
Comments suppressed due to low confidence (1)
tests/api-tests/test_api_health.py:6
- [nitpick] Consider using a consistent working directory setup (e.g., using ".." as in other test files) to ensure a uniform test environment.
root_dir = rootutils.setup_root(".", indicator=".project-root", pythonpath=True)
|
|
||
| response = requests.request("POST", url, headers=headers, data=payload) | ||
|
|
||
| assert response.status_code == 400 |
There was a problem hiding this comment.
The docstring of test_retrosynthesis_fail expects a 500 status code, but the assertion checks for 400. Please align the expected status code in the docstring and the test.
|
|
||
| response = requests.request("POST", url, headers=headers, data=payload) | ||
|
|
||
| assert response.status_code == 400 |
There was a problem hiding this comment.
The docstring of test_rerun_retro_fail expects a 500 status code while the assertion checks for 400. Update either the docstring or the assertion for consistency.
tests/variables_test.py
Outdated
| } | ||
|
|
||
|
|
||
| # "http://ec2-3-17-191-215.us-east-2.compute.amazonaws.com:5000" |
There was a problem hiding this comment.
[nitpick] The commented BASE_URL does not match the new BASE_URL set below. Consider updating or removing the outdated comment to avoid confusion.
| # "http://ec2-3-17-191-215.us-east-2.compute.amazonaws.com:5000" | |
| # "http://ec2-18-220-15-234.us-east-2.compute.amazonaws.com:5000" |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces structured tests for both API endpoints and LLM functionality, along with new CI workflows and some cleanup to improve test consistency. Key changes include:
- Addition of new constants and test files for various API endpoints and LLM parsers.
- Updates to the root directory initialization in several LLM test files.
- Inclusion of CI workflows for linting, LLM tests, and API tests.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/variables_test.py | Added new model constants and API configuration variables used by tests. |
| tests/llm-tests/test_llm_parsers.py, test_llm.py, test_adv_prompt.py | Updated root directory setup from "." to ".." to ensure correct project root resolution. |
| tests/api-tests/* | Added comprehensive API tests for both retrosynthesis and rerun endpoints with success and failure scenarios. |
| .github/workflows/* | Introduced new CI workflows for Python linting, LLM tests, and API tests. |
Uh oh!
There was an error while loading. Please reload this page.