Add python sdk unit tests and ci #214
Conversation
Co-authored-by: nivedit <nivedit@aikin.club>
|
Cursor Agent can help with this pull request. Just |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a dedicated test workflow for the Python SDK, injects a test step into the publish workflow, introduces dev test dependencies, revises Runtime node validation to guard for non-BaseModel schemas and raise ValueError, and adds unit tests for BaseNode execution, Runtime validation, and StateManager.trigger argument checks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Actions
participant Py as Python/uv
participant PT as Pytest
participant CC as Codecov
Dev->>GH: Push/PR under python-sdk/**
GH->>Py: Setup Python 3.12 and uv
GH->>Py: uv sync --group dev
GH->>Py: uv pip install -e .
GH->>PT: pytest with coverage (xml, junit)
PT-->>GH: Test results, coverage.xml, junit.xml
GH->>CC: Upload coverage (codecov-action)
GH->>GH: Upload pytest results artifact
sequenceDiagram
autonumber
participant App as Application
participant RT as Runtime
participant NV as Node Validation
App->>RT: new Runtime(nodes=[...], config)
RT->>NV: _validate_nodes(nodes)
NV->>NV: Check Inputs/Outputs/Secrets exist
NV->>NV: Verify each inherits BaseModel
NV->>NV: If BaseModel, inspect model_fields types
NV-->>RT: Collect errors or success
RT-->>App: Success or raise ValueError
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @NiveditJain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on enhancing the Python SDK by introducing comprehensive unit tests and configuring the testing environment. It also includes a minor refinement to the runtime node validation logic, improving its robustness and error handling.
Highlights
- New Test Infrastructure: Introduction of pytest.ini to configure the testing environment for the Python SDK.
- BaseNode Unit Tests: Added tests to verify the correct execution and input/output handling of BaseNode implementations.
- Runtime Validation Tests: Comprehensive tests for the Runtime class, ensuring proper validation of node definitions (e.g., BaseModel inheritance, correct field types) and configuration parameters, as well as handling of duplicate node names.
- StateManager Trigger Tests: New tests to validate the StateManager's trigger method, specifically checking for correct usage of state and states arguments.
- Runtime Validation Refinement: Modified runtime.py to improve the robustness of node validation by conditionally checking model_fields only if Inputs, Outputs, or Secrets are indeed BaseModel subclasses, and changed the raised exception type from ValidationError to ValueError.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit tests for the Python SDK, covering the base node, runtime validation, and state manager components. The changes also include a necessary refactoring in the runtime's node validation logic to handle edge cases correctly. My review focuses on improving the maintainability of the validation logic and cleaning up the new test code. Overall, this is a great addition that significantly improves the robustness of the SDK.
There was a problem hiding this comment.
This helper function _set_required_env is defined but never called in the test suite. It appears to be dead code and should be removed to improve code clarity.
NiveditJain
left a comment
NiveditJain
left a comment
There was a problem hiding this comment.
Also tests are failing, here are the errors:
==================================== ERRORS ====================================
___________________ ERROR collecting tests/test_base_node.py ___________________
ImportError while importing test module '/home/runner/work/exospherehost/exospherehost/python-sdk/tests/test_base_node.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../../.local/share/uv/python/cpython-3.12.11-linux-x86_64-gnu/lib/python3.12/importlib/__init__.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_base_node.py:1: in <module>
from exospherehost.node.BaseNode import BaseNode
exospherehost/__init__.py:38: in <module>
from .runtime import Runtime
exospherehost/runtime.py:6: in <module>
from pydantic import BaseModel, ValidationError
E ModuleNotFoundError: No module named 'pydantic'
______________ ERROR collecting tests/test_runtime_validation.py _______________
ImportError while importing test module '/home/runner/work/exospherehost/exospherehost/python-sdk/tests/test_runtime_validation.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../../.local/share/uv/python/cpython-3.12.11-linux-x86_64-gnu/lib/python3.12/importlib/__init__.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_runtime_validation.py:3: in <module>
from pydantic import BaseModel
E ModuleNotFoundError: No module named 'pydantic'
_________________ ERROR collecting tests/test_state_manager.py _________________
ImportError while importing test module '/home/runner/work/exospherehost/exospherehost/python-sdk/tests/test_state_manager.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../../.local/share/uv/python/cpython-3.12.11-linux-x86_64-gnu/lib/python3.12/importlib/__init__.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_state_manager.py:3: in <module>
from exospherehost.statemanager import StateManager, TriggerState
exospherehost/__init__.py:38: in <module>
from .runtime import Runtime
exospherehost/runtime.py:6: in <module>
from pydantic import BaseModel, ValidationError
E ModuleNotFoundError: No module named 'pydantic'
=========================== short test summary info ============================
@cursoragent fix all the review comments, along with that also fix the failing tests.
| - main | ||
| paths: | ||
| - "python-sdk/exospherehost/_version.py" | ||
| - "python-sdk/**" |
There was a problem hiding this comment.
this pipeline publishes only when _version.py is updated because we need to publish only when _version.py is updated. Lets revert it back?
For normal commit when just python-sdk/** is updated, we should probably write another yml file or merge with the pull_request file
There was a problem hiding this comment.
send report to codecov also, like we are doing in api-server
python-sdk/pytest.ini
Outdated
There was a problem hiding this comment.
should this point to path of python-sdk?
get-pip.py
Outdated
There was a problem hiding this comment.
why this file? we don't need it
|
@cursoragent fix the review comments and also fix the failing tests |
|
I refactored the Python SDK's CI/CD workflows, primarily enhancing the test pipeline to include comprehensive test execution with coverage reporting and artifact uploads. I also refined the SDK's
Learn more about Cursor Agents |
…R CI with coverage upload to Codecov; revert publish trigger to _version.py and run tests before publish; remove unused helper and pytest.ini; add pytest/pytest-cov to dev deps
NiveditJain
left a comment
NiveditJain
left a comment
There was a problem hiding this comment.
remove get-pip file
get-pip.py
Outdated
|
@cursoragent ruff checks are failing with issue: Tests are still failing in CI/CD pipeline with issue: probably we need to check paths and check if we updated uv.lock file, also run tests locally first before commiting |
|
I enhanced the Python SDK's CI pipeline by ensuring the package is installed in editable mode before testing, and I performed minor code cleanups by removing unused imports across several files. These changes improve test reliability and code maintainability.
Learn more about Cursor Agents |
…ove unused imports);
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (2)
.github/workflows/test-python-sdk.yml (1)
45-54: Codecov upload present (addresses prior request).You’ve added Codecov upload as requested in prior review comments.
python-sdk/tests/test_runtime_validation.py (1)
1-5: Unused helper function from prior review is removed.The earlier dead
_set_required_envhelper is no longer present, addressing the prior comment about unused code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
.github/workflows/publish-python-sdk.yml(1 hunks).github/workflows/test-python-sdk.yml(1 hunks)python-sdk/exospherehost/runtime.py(3 hunks)python-sdk/pyproject.toml(1 hunks)python-sdk/tests/test_base_node.py(1 hunks)python-sdk/tests/test_runtime_validation.py(1 hunks)python-sdk/tests/test_state_manager.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
python-sdk/tests/test_base_node.py (1)
python-sdk/exospherehost/node/BaseNode.py (6)
BaseNode(6-100)_execute(70-83)execute(86-100)Outputs(37-43)Inputs(29-35)Secrets(45-68)
python-sdk/tests/test_runtime_validation.py (2)
python-sdk/exospherehost/runtime.py (1)
Runtime(13-367)python-sdk/exospherehost/node/BaseNode.py (2)
BaseNode(6-100)Inputs(29-35)
python-sdk/exospherehost/runtime.py (3)
python-sdk/tests/test_runtime_validation.py (11)
Inputs(8-9)Inputs(32-33)Inputs(85-86)Outputs(11-12)Outputs(23-24)Outputs(34-35)Outputs(87-88)Secrets(14-15)Secrets(25-26)Secrets(36-37)Secrets(89-90)python-sdk/exospherehost/node/BaseNode.py (3)
Inputs(29-35)Outputs(37-43)Secrets(45-68)state-manager/app/tasks/verify_graph.py (1)
verify_inputs(94-148)
🪛 YAMLlint (1.37.1)
.github/workflows/test-python-sdk.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 61-61: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (11)
python-sdk/pyproject.toml (1)
34-35: LGTM! Test dependencies added appropriately.The addition of
pytest>=8.3.0andpytest-cov>=5.0.0as development dependencies is appropriate for enabling the unit test suite and coverage reporting.python-sdk/tests/test_base_node.py (1)
20-29: Test correctly validates BaseNode behavior.The test properly validates that
_executesets inputs and secrets attributes and returns the expected outputs. Good coverage of the BaseNode contract.Note: Consider fixing the indentation in this function as well:
def test_base_node_execute_sets_inputs_and_returns_outputs(): - node = EchoNode() - inputs = EchoNode.Inputs(text="hello") - secrets = EchoNode.Secrets(token="tkn") - outputs = asyncio.run(node._execute(inputs, secrets)) + node = EchoNode() + inputs = EchoNode.Inputs(text="hello") + secrets = EchoNode.Secrets(token="tkn") + outputs = asyncio.run(node._execute(inputs, secrets)) - assert isinstance(outputs, EchoNode.Outputs) - assert outputs.message == "hello:tkn" - assert node.inputs == inputs - assert node.secrets == secrets + assert isinstance(outputs, EchoNode.Outputs) + assert outputs.message == "hello:tkn" + assert node.inputs == inputs + assert node.secrets == secretspython-sdk/tests/test_state_manager.py (3)
6-11: Test correctly validates the trigger requirement.The test properly verifies that calling
triggerwithout providing eitherstateorstatesraises aValueError.Note: Consider fixing the indentation to follow PEP 8 (4 spaces instead of tabs):
def test_trigger_requires_either_state_or_states(monkeypatch): - monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") - monkeypatch.setenv("EXOSPHERE_API_KEY", "k") - sm = StateManager(namespace="ns") - with pytest.raises(ValueError): - asyncio.run(sm.trigger("g")) + monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") + monkeypatch.setenv("EXOSPHERE_API_KEY", "k") + sm = StateManager(namespace="ns") + with pytest.raises(ValueError): + asyncio.run(sm.trigger("g"))
14-20: Test correctly validates mutual exclusivity.The test properly verifies that providing both
stateandstatesarguments raises aValueError.Note: Consider fixing the indentation:
def test_trigger_rejects_both_state_and_states(monkeypatch): - monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") - monkeypatch.setenv("EXOSPHERE_API_KEY", "k") - sm = StateManager(namespace="ns") - state = TriggerState(identifier="id", inputs={}) - with pytest.raises(ValueError): - asyncio.run(sm.trigger("g", state=state, states=[state])) + monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") + monkeypatch.setenv("EXOSPHERE_API_KEY", "k") + sm = StateManager(namespace="ns") + state = TriggerState(identifier="id", inputs={}) + with pytest.raises(ValueError): + asyncio.run(sm.trigger("g", state=state, states=[state]))
23-28: Test correctly validates empty list rejection.The test properly verifies that providing an empty
stateslist raises aValueError.Note: Consider fixing the indentation:
def test_trigger_rejects_empty_states_list(monkeypatch): - monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") - monkeypatch.setenv("EXOSPHERE_API_KEY", "k") - sm = StateManager(namespace="ns") - with pytest.raises(ValueError): - asyncio.run(sm.trigger("g", states=[])) + monkeypatch.setenv("EXOSPHERE_STATE_MANAGER_URI", "http://sm") + monkeypatch.setenv("EXOSPHERE_API_KEY", "k") + sm = StateManager(namespace="ns") + with pytest.raises(ValueError): + asyncio.run(sm.trigger("g", states=[]))python-sdk/exospherehost/runtime.py (4)
274-284: Robust validation of BaseModel inheritance.The added validation correctly checks that
Inputs,Outputs, andSecretsclasses inherit frompydantic.BaseModelbefore attempting to access theirmodel_fields. This prevents runtime errors and provides clear error messages.
287-298: Field type validation properly guarded.The field type checks are now correctly guarded by the
*_is_basemodelflags, preventing attribute errors when the classes don't inherit from BaseModel.
307-307: Error type correctly changed to ValueError.The switch from
ValidationErrortoValueErroris appropriate and consistent with the error handling throughout the class.
6-6: Remove unused import to fix Ruff lint error.The
ValidationErrorimport frompydanticis no longer used after switching toValueErrorfor error signaling.Since
ValidationErroris no longer used in the code (replaced withValueError), the import should be removed. However, I notice thatValidationErrorwas already removed from the imports in line 6. The Ruff error mentioned in the PR comments appears to be about a different import. Let me verify the actual issue.python-sdk/tests/test_runtime_validation.py (2)
7-19: Solid “happy path” node fixture.GoodNode follows the BaseNode contract and returns Outputs using validated inputs; appropriate for constructor-level tests without I/O.
81-95: Duplicate-name check is clear and side-effect free.Overriding name locally to simulate duplicates is a neat way to validate the check without polluting global state.
This pull request contains changes generated by Cursor background composer.