Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add plugin tests #1096

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ jobs:
export E2B_API_KEY=${{ secrets.E2B_API_KEY_STAGING }}

tox -e test -- -m 'not e2e and not swe'
- name: Plugin tests
if: matrix.python-version == '3.10'
run: |
tox -e plugins
- if: matrix.python-version == '3.10'
name: Upload test results to Codecov
uses: codecov/test-results-action@v1
Expand Down
4 changes: 3 additions & 1 deletion python/composio/tools/local/codeanalysis/embedder.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from typing import Any, Dict, List

from deeplake.core.vectorstore.deeplake_vectorstore import DeepLakeVectorStore
from sentence_transformers import SentenceTransformer

from composio.tools.local.codeanalysis.constants import (
CODE_MAP_CACHE,
Expand Down Expand Up @@ -49,6 +48,9 @@ def get_vector_store(repo_name: str, overwrite: bool = True) -> DeepLakeVectorSt

class Embedding:
def __init__(self):
# pylint: disable=import-outside-toplevel
from sentence_transformers import SentenceTransformer

self.model = SentenceTransformer(EMBEDDER)

def compute(self, texts: List[str]) -> List[List[float]]:
Expand Down
58 changes: 58 additions & 0 deletions python/tests/test_tools/test_plugins.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import composio_openai
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider grouping related imports together and removing unused ones:

from composio import Action, App  # Action is unused
from composio.client.enums.base import ActionData  # ActionData is unused
from composio.tools.local.base import action

# Group plugin imports
import composio_openai
import composio_crewai  # Unused
import composio_langchain  # Unused
import composio_llamaindex  # Unused

This will make the imports cleaner and more maintainable.



def test_openai_toolset():
toolset = composio_openai.ComposioToolSet()

@composio_openai.action(toolname="my_tool")
def create_draft(
message_body: str,
thread_id: str | None = None,
) -> dict:
"""
Create a draft reply to a specific Gmail thread

:param thread_id: The ID of the thread to which the reply is to be drafted
:param message_body: The content of the draft reply
:return draft: The created draft details
"""
_ = message_body, thread_id
return {}

tools = toolset.get_tools(actions=[create_draft])
assert tools == [
{
"function": {
"description": "Create a draft reply to a specific Gmail thread",
"name": "MYTOOL_CREATE_DRAFT",
"parameters": {
"properties": {
"message_body": {
"description": "The content of the draft reply. Please provide a "
"value of type string. This parameter is required.",
"title": "Message Body",
"type": "string",
},
"thread_id": {
"description": "The ID of the thread to which the reply is to be "
"drafted. Please provide a value of type string.",
"title": "Thread Id",
# TODO: this seems wrong, why both type and anyof
"type": "string",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of both type and anyOf for thread_id is inconsistent. Consider removing the type key or ensure it aligns with the anyOf specification.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redundant type definition here could be simplified. Since thread_id is an optional string parameter, we could either:

  1. Remove the type: string and keep only the anyOf definition, or
  2. Use type: ["string", "null"] instead

The current implementation with both type and anyOf is redundant and could cause confusion in schema validation.

"default": None,
"anyOf": [
{"type": "string"},
{"type": "null"},
],
},
},
"required": [
"message_body",
],
"title": "CreateDraftRequest",
"type": "object",
},
},
"type": "function",
},
Comment on lines +38 to +57

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Bug Fix:

Redundant Type Definition in 'thread_id' Field
The current schema for the 'thread_id' field uses both 'type' and 'anyOf', which is redundant and can lead to confusion. The 'anyOf' construct already specifies that the field can be either a string or null, making the 'type' declaration unnecessary. Removing the 'type' field will simplify the schema and prevent potential validation issues. This change will ensure clarity and maintainability of the code. 🛠️

🔧 Suggested Code Diff:
- "type": "string",
  "anyOf": [
      {"type": "string"},
      {"type": "null"},
  ],
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
"drafted. Please provide a value of type string.",
"title": "Thread Id",
# TODO: this seems wrong, why both type and anyof
"type": "string",
"default": None,
"anyOf": [
{"type": "string"},
{"type": "null"},
],
},
},
"required": [
"message_body",
],
"title": "CreateDraftRequest",
"type": "object",
},
},
"type": "function",
},
{
"thread_id": {
"description": "The ID of the thread to which the reply is to be drafted. Please provide a value of type string.",
"title": "Thread Id",
"default": None,
"anyOf": [
{"type": "string"},
{"type": "null"}
]
}
}

Comment on lines +38 to +57

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Bug Fix:

Redundant Schema Definition in 'CreateDraftRequest'
The current schema definition for 'CreateDraftRequest' includes both a 'type' field and an 'anyOf' field, which is redundant and can lead to confusion in validation logic. If the intention is to allow both strings and null values, the 'type' field should be removed, and 'anyOf' should be used exclusively. Alternatively, if only strings are allowed, the 'anyOf' field should be removed. This will ensure clarity and correctness in the schema definition.

🔧 Suggested Code Diff:
- "type": "string",
+ // Remove the 'type' field if allowing both strings and null values
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
"drafted. Please provide a value of type string.",
"title": "Thread Id",
# TODO: this seems wrong, why both type and anyof
"type": "string",
"default": None,
"anyOf": [
{"type": "string"},
{"type": "null"},
],
},
},
"required": [
"message_body",
],
"title": "CreateDraftRequest",
"type": "object",
},
},
"type": "function",
},
{
"CreateDraftRequest": {
"type": "object",
"properties": {
"message_body": {
"anyOf": [
{"type": "string"},
{"type": "null"}
],
"default": None
}
},
"required": [
"message_body"
],
"title": "CreateDraftRequest"
}
}

tushar-composio marked this conversation as resolved.
Show resolved Hide resolved
]
30 changes: 21 additions & 9 deletions python/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,29 @@ commands =

; TODO: Extract plugin tests separately
; Installing separately because of the dependency conflicts
uv pip install plugins/langchain --no-deps
uv pip install plugins/langchain

pytest -vvv -rfE --doctest-modules composio/ tests/ swe/tests --junitxml=junit.xml --cov=composio --cov=examples --cov=swe --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov-config=.coveragerc {posargs}
pytest -vvv -rfE --doctest-modules composio/ tests/ swe/tests --ignore tests/test_tools/test_plugins.py --junitxml=junit.xml --cov=composio --cov=examples --cov=swe --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov-config=.coveragerc {posargs}

; uv pip install plugins/autogen
; uv pip install plugins/claude
; uv pip install plugins/crew_ai
; uv pip install plugins/griptape
; uv pip install plugins/julep
; uv pip install plugins/lyzr
; uv pip install plugins/openai

Comment on lines 121 to +128

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential Issue:

Ensure Comprehensive Test Coverage for Excluded Test File
The recent change excludes tests/test_tools/test_plugins.py from the main test suite, which could lead to gaps in test coverage. It's crucial to ensure that this test file is included in a new dedicated job for plugin tests, especially under Python 3.10. This will help maintain comprehensive test coverage and prevent potential bugs from going unnoticed.

Actionable Steps:

  • Verify that tests/test_tools/test_plugins.py is included in a separate test job.
  • Ensure that the new job runs under Python 3.10 to maintain compatibility and coverage.
  • Regularly review test coverage reports to confirm no tests are omitted inadvertently.

[testenv:plugins]
setenv =
CI={env:CI}
COMPOSIO_API_KEY={env:COMPOSIO_API_KEY}
COMPOSIO_BASE_URL={env:COMPOSIO_BASE_URL}
deps =
.
plugins/autogen
plugins/claude
plugins/crew_ai
plugins/griptape
plugins/julep
plugins/langchain
plugins/langgraph
plugins/llamaindex
plugins/openai
commands =
pytest -vv tests/test_tools/test_plugins.py {posargs}

; Linter config

Expand Down
Loading