Skip to content

Conversation

@ryoppippi
Copy link
Member

@ryoppippi ryoppippi commented Dec 22, 2025

Summary

  • Remove unused _matches_filter method from StackOneToolSet
  • Add comprehensive tests for models.py and toolset.py modules
  • Improve overall test coverage from 76% to 87%

Changes

Code cleanup

  • Removed _matches_filter method which was defined but never called in source code

New tests for models.py (78% → 95%)

  • validate_method unsupported HTTP method error
  • ExecuteConfig validation
  • Parameter location handling (PATH, QUERY, BODY)
  • HTTP error handling (JSON and text response bodies)
  • RequestError handling
  • Non-dict response wrapping
  • OpenAI conversion (enum, array, object, non-dict properties)
  • LangChain conversion (number, integer, boolean types)
  • Async _arun method
  • Tools container iteration and account ID methods

New tests for toolset.py (52% → 77%)

  • _StackOneRpcTool execution (basic, JSON string, payloads)
  • Header handling (Authorization stripping, None value skipping)
  • Argument parsing edge cases

Future work

  • Add tests for integrations/langgraph.py (currently 0% coverage)
  • The remaining uncovered lines in toolset.py are MCP communication code (_run_async, _fetch_mcp_tools) which are mocked in tests

Test plan

  • All existing tests pass
  • New tests pass
  • Lint and type checks pass
  • Coverage improved to 87%

Summary by cubic

Improved test coverage from 76% to 87% by adding thorough tests across models and toolset, and removing an unused filter helper for cleaner code.

  • Refactors

    • Removed unused _matches_filter from StackOneToolSet; existing _filter_by_provider and _filter_by_action handle filtering.
  • Tests

    • models.py: validate_method, ExecuteConfig validation, parameter locations (PATH/QUERY/BODY), HTTP error handling, non-dict response wrapping.
    • Conversion: OpenAI and LangChain conversions for enums, arrays, objects, and basic types; async _arun.
    • Tools container: iteration and account ID getters/setters.
    • toolset.py: _StackOneRpcTool execution (JSON string and structured payloads), header handling (strip Authorization, skip None), and argument parsing edge cases.

Written for commit da130b2. Summary will update automatically on new commits.

Remove the _matches_filter method from StackOneToolSet as it was
never called in the source code (only used in tests). This reduces
code complexity and improves test coverage percentage.

The existing _filter_by_provider and _filter_by_action methods
provide equivalent functionality and are actually used in fetch_tools.
Add comprehensive tests for previously uncovered code paths:

models.py (78% -> 95%):
- validate_method unsupported HTTP method error
- ExecuteConfig validation
- Parameter location handling (PATH, QUERY, BODY)
- HTTP error handling (JSON and text response bodies)
- RequestError handling
- Non-dict response wrapping
- OpenAI conversion (enum, array, object, non-dict properties)
- LangChain conversion (number, integer, boolean types)
- Async _arun method
- Tools container iteration and account ID methods

toolset.py (52% -> 77%):
- _StackOneRpcTool execution (basic, JSON string, payloads)
- Header handling (Authorization stripping, None value skipping)
- Argument parsing edge cases

Overall coverage improved from 76% to 87%.
Copilot AI review requested due to automatic review settings December 22, 2025 13:19
@ryoppippi ryoppippi changed the title test: improve test coverage from 76% to 87% test: improve coverage for models and toolset modules (76% → 87%) Dec 22, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves test coverage from 76% to 87% by removing unused code and adding comprehensive tests for the models.py and toolset.py modules.

  • Removed the unused _matches_filter method from StackOneToolSet and its associated tests
  • Added extensive test coverage for StackOneTool execution, parameter handling, error scenarios, and framework conversions
  • Added comprehensive tests for _StackOneRpcTool RPC execution with various payload types and edge cases

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
stackone_ai/toolset.py Removed unused _matches_filter method (28 lines)
tests/test_toolset.py Removed tests for deleted _matches_filter method (29 lines)
tests/test_tool_calling.py Added 199 lines of comprehensive tests for _StackOneRpcTool including execution, header handling, and argument parsing
tests/test_models.py Added 536 lines of tests covering validation, execution edge cases, error handling, OpenAI/LangChain conversions, and Tools container functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

Copy link
Contributor

@glebedel glebedel left a comment

Choose a reason for hiding this comment

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

LGTM

self.base_url = base_url or DEFAULT_BASE_URL
self._account_ids: list[str] = []

def _matches_filter(self, tool_name: str, filter_pattern: str | list[str]) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this not used for glob filtering at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

we use _filter_by_action function instead

@ryoppippi ryoppippi merged commit 663b7e4 into main Dec 22, 2025
21 checks passed
@ryoppippi ryoppippi deleted the improve-test-coverage branch December 22, 2025 13:45
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.

3 participants