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

Add test coverage #141

Merged
merged 29 commits into from
Jun 14, 2024
Merged

Add test coverage #141

merged 29 commits into from
Jun 14, 2024

Conversation

angrybayblade
Copy link
Collaborator

@angrybayblade angrybayblade commented Jun 11, 2024

This PR intends to add tests for

  • Examples
  • CLI tools
  • E2E Integrations
  • Unittests

🚀 This description was created by Ellipsis for commit 556d505

Summary:

This PR enhances test coverage and updates configurations for various components, with significant refactoring and updates to CI workflow, including new tests and configuration refinements.

Key points:

  • Enhanced test coverage for enums, examples, CLI tools, E2E integrations, and unittests
  • Added and updated tests across multiple components
  • Introduced new tests for enums and examples, updated enum properties
  • Added specific tests for E2E testing of plugin demos, client base, storage helper, and schema helpers
  • Updated configurations and refactored code
  • Disabled static type check in CI workflow
  • Updated and refined CLI test helpers and command groups
  • Modified task descriptions and test documentation strings
  • Removed outdated test files and refactored existing tests for clarity
  • Updated composio/local_tools/local_workspace/tests/test_workspace.py to skip tests in CI environment
  • Modified composio/utils/url.py to fix a typo in the error message
  • Added tests/test_cli/test_actions.py for testing CLI actions
  • Updated tests/test_example.py to skip tests in CI environment to avoid excessive API usage
  • Updated tests/test_utils/test_url.py to test get_web_url function
  • Updated tox.ini to refine test configurations
  • Added --active option to connections command in composio/cli/connections.py to filter active connections
  • Updated _connections and _get functions in composio/cli/connections.py to improve output formatting
  • Created tests/test_cli/test_connections.py to test connections command group
  • Added tests for listing all connections and getting details of a specific connection in tests/test_cli/test_connections.py
  • Updated composio/client/collections.py to make logo fields optional in AppModel, TriggerModel, ActionModel, and IntegrationModel
  • Created tests/test_cli/test_integrations.py to test the integrations command group
  • Updated composio/client/collections.py to rename _trigger_names_str to trigger_names_str
  • Created tests/test_client/test_collections.py to test trigger_names_str function

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 360de61 in 1 minute and 4 seconds

More details
  • Looked at 202 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_jgG73L9Z2piwXGk0


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

tests/test_example.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 4b638e3 in 1 minute and 19 seconds

More details
  • Looked at 207 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_client/test_enum.py:5
  • Draft comment:
    The import of unittest is unnecessary as the tests are written using pytest style. Consider removing this import to avoid confusion.
  • Reason this comment was not posted:
    Confidence of 20% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_15DLWRuwtqbNktVA


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on eb93e7a in 1 minute and 47 seconds

More details
  • Looked at 39 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_example.py:29
  • Draft comment:
    Consider adding a default value or a more descriptive error handling for OPENAI_API_KEY and COMPOSIO_API_KEY if they are not set. This will make the tests more robust and the error messages clearer if the environment variables are missing.
OPENAI_API_KEY = os.environ.get("OPENAI_API_KEY", "")
COMPOSIO_API_KEY = os.environ.get("COMPOSIO_API_KEY", "")
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_B8JQ3P5UzirB5ERl


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on e26da5c in 1 minute and 13 seconds

More details
  • Looked at 105 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_EzZFRPM7IWeQI2Ox


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

tests/test_cli/test_apps.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on c91c3d6 in 1 minute and 17 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_yOyvFvBMAEUkd5nU


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

tests/test_client/test_enum.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 2c21f68 in 1 minute and 28 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_0uFXYNkJiivmqigv


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

tests/test_client/test_enum.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7861be6 in 1 minute and 46 seconds

More details
  • Looked at 73 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. .github/workflows/common.yml:59
  • Draft comment:
    The static type check (mypy) is commented out. If this is intentional, please provide a reason in the comments or documentation. Otherwise, consider re-enabling it to ensure type safety.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.
2. tests/test_client/test_enum.py:9
  • Draft comment:
    The SKIP_CI decorator has been removed from the enum tests. If the CI timeout issue is resolved, please confirm in the comments. Otherwise, consider reinstating the decorator or implementing an alternative solution to handle the timeouts.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_OR28MCHmwrMZOoRK


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 10c0e4f in 1 minute and 55 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/common.yml:71
  • Draft comment:
    The increase in timeout for the test job from 15 to 90 minutes seems excessive. Consider optimizing the tests to reduce the required time, or provide justification for such a long duration to avoid potential resource wastage.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_CrDuQ5wN1cRm7brr


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on e2f242d in 2 minutes and 14 seconds

More details
  • Looked at 35 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_cli/test_apps.py:38
  • Draft comment:
    Modifying actual files during testing is risky and can lead to unintended side effects. Consider using a mock or a temporary file instead of directly modifying enums.__file__.
  • Reason this comment was not posted:
    Confidence of 40% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_xL6I0YhfZuORnaD4


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 112b414 in 3 minutes and 31 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tox.ini:88
  • Draft comment:
    The PR description states that static type checking has been disabled in the CI workflow, but the mypy test environment is still present here. If static type checking is indeed disabled, consider removing this section to avoid confusion and maintain consistency with the PR's intent.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_DwuApr4yDrXfUgxh


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on b0f8927 in 1 minute and 57 seconds

More details
  • Looked at 179 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_zU87AzyuqhtoHq8n


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ValueError,
match=(
"Incorrect format for base_url: http://url. Format should "
"be on of follwing {https://backend.composio.dev/api, "
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo in the error message assertion. It should be corrected for clarity and professionalism.

Suggested change
"be on of follwing {https://backend.composio.dev/api, "
"be one of the following {https://backend.composio.dev/api, "

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 8c35e8a in 2 minutes and 33 seconds

More details
  • Looked at 250 lines of code in 8 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_client/test_base.py:19
  • Draft comment:
    The test name test_raise_if_required does not correspond to any method in the _Collection class. It seems to be testing the get method instead. Consider renaming the test to reflect the actual method being tested, such as test_get_raises_http_error_on_404.
  • Reason this comment was not posted:
    Confidence of 40% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_lvbtZVQwSnFXLx5E


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on dc8d170 in 4 minutes and 25 seconds

More details
  • Looked at 34 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_example.py:31
  • Draft comment:
    It's risky to directly use environment variables without checking if they are set. Consider adding a check before the tests run to ensure all necessary environment variables are present, and skip tests or fail with a clear message if they are not.

OPENAI_API_KEY = os.environ.get("OPENAI_API_KEY")
if not OPENAI_API_KEY:
pytest.skip("OPENAI_API_KEY is not set")
COMPOSIO_API_KEY = os.environ.get("COMPOSIO_API_KEY")
if not COMPOSIO_API_KEY:
pytest.skip("COMPOSIO_API_KEY is not set")

  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_LJNiQ1PHreK4TQIl


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on b39885f in 1 minute and 59 seconds

More details
  • Looked at 27 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. plugins/julep/composio_julep/toolset.py:38
  • Draft comment:
    Changing the runtime attribute to _runtime makes it private and encapsulated, which is a good practice. However, ensure that all references to runtime in the codebase are updated to _runtime to avoid breaking changes.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_KW3lJ5fU9U6qJOmo


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on e39e3a6 in 2 minutes and 8 seconds

More details
  • Looked at 26 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/common.yml:101
  • Draft comment:
    The PR description states that static type checking has been disabled, but the workflow file still includes the mypy job. If the intention is to disable it, you should remove or comment out this step.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_4THLeGW3lfMf3vu3


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on cd5d511 in 1 minute and 36 seconds

More details
  • Looked at 135 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tox.ini:90
  • Draft comment:
    Disabling static type checks in the CI workflow could lead to type-related bugs slipping through. Please ensure there are adequate measures in place to catch potential issues.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_7oGqXbWu9abERw4L


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on abe57c0 in 2 minutes and 1 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. .github/workflows/common.yml:101
  • Draft comment:
    The environment variable name COMPOSIO_API_KEY_STAGING has been changed to COMPOSIO_API_KEY. Ensure that this change is consistent across the entire codebase and any related documentation.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_57QQirVkuAY43MdQ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 2e05f63 in 1 minute and 44 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tox.ini:79
  • Draft comment:
    The PR description states that static type checking has been disabled in the CI workflow, but the mypy configuration is still present here. Please clarify or update the PR to reflect the actual changes made.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_ApbH4uwrIvsJFpEp


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 0a0a5d1 in 1 minute and 54 seconds

More details
  • Looked at 81 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_KQqCFIAHKqzOZKv3


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

"""Test list connections."""
result = self.run("connections")

assert result.exit_code == 0, result.stderr
Copy link
Contributor

Choose a reason for hiding this comment

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

The test test_list_all is checking for output strings that do not match the actual output format used in the _connections function. Update the test to check for the correct strings:

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on cc54b7f in 2 minutes and 22 seconds

More details
  • Looked at 44 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. tests/test_cli/test_actions.py:16
  • Draft comment:
    There is a typo in the parameter name exptected_outputs; it should be expected_outputs to maintain consistency and avoid potential confusion.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_CSH9H0HghPSoo5lv


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on b92caaa in 2 minutes and 6 seconds

More details
  • Looked at 80 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_U5tto1str9Rxe4qW


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

tests/test_cli/test_integrations.py Show resolved Hide resolved
.github/workflows/common.yml Outdated Show resolved Hide resolved
tests/test_client/test_triggers.py Show resolved Hide resolved
@angrybayblade angrybayblade merged commit 711a062 into master Jun 14, 2024
5 checks passed
@angrybayblade angrybayblade deleted the test/cli branch June 14, 2024 04:44
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.

2 participants