chore: add tests #103
chore: add tests #103
Conversation
github-actions
bot
added
t-ai
…Server class creation
jirispilka
added
the
high priority
* add help tool * Update src/tools/helpers.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: move tool text into a constant * fix: helpTool text * Update src/tools/helpers.ts Co-authored-by: Jiří Spilka <jiri.spilka@apify.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jiri Spilka <jirka.spilka@gmail.com> Co-authored-by: Jiří Spilka <jiri.spilka@apify.com>
| beforeEach(async () => { | ||
| // same as in main.ts | ||
| // TODO: unify | ||
| server = createActorMCPServer(); |
There was a problem hiding this comment.
server is created twice. I would really remove the createActorMCPServer function. Can you handle TODO as well?
There was a problem hiding this comment.
Removed the createActorMCPServer wrapper and TODO since I copied the logic from main.ts so it's unified.
server is created twice.
You mean there is separate server for each describe block? If so that is correct, I wanted to split the different transport tests so they can run in parallel so it spawns two server instances on different ports. Since the Actor MCP server logic is coupled and only single-tenant the actual tests (it blocks) must run in serial.
jirispilka
left a comment
jirispilka
left a comment
There was a problem hiding this comment.
I didn’t have time to look at the test in detail,
but overall, it looks good. I’ve left a few comments that should be addressed before merging.
…rectly, removed duplicate tools.actor tests and renamed actor.test.ts, removed old TODOs
for each state to fix MaxListenersExceededWarning
There was a problem hiding this comment.
Pull Request Overview
This PR adds enhanced test coverage across unit and integration tests while also updating some helper functionality and default configurations.
- Introduces a new test timeout in the Vitest configuration
- Adds multiple unit tests for utilities and updates import paths
- Implements a help tool in the MCP server and adjusts default tool configurations
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Added a test timeout setting |
| tests/unit/*.test.ts & tests/integration/stdio.test.ts | Added and updated unit and integration tests |
| tests/helpers.ts | Introduced helper functions for various client transports |
| src/tools/helpers.ts | Added a help tool with extensive inline documentation |
| src/mcp/server.ts | Updated default tools and added reset/close methods |
| src/const.ts | Updated default actors and helper tool configuration |
| tests/actor-server-test.ts | Removed obsolete actor server tests |
| tests/README.md | Added documentation for running tests |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (2)
src/const.ts:33
- Verify that the removal of the default actor 'apify/instagram-scraper' (and related entries) is intentional, as this alters the expected configuration of default actors.
- 'apify/instagram-scraper',
src/tools/helpers.ts:12
- [nitpick] Consider externalizing the lengthy inline help text into a dedicated documentation file or module to improve maintainability and readability of the source code.
const HELP_TOOL_TEXT = `Apify MCP server help: ...
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
jirispilka
left a comment
jirispilka
left a comment
There was a problem hiding this comment.
Thanks! The actor.server.test.ts and stdio.test.ts files contain a lot of duplicated code. You could refactor this by extracting the shared logic into a function and simply passing the client as a parameter. This would significantly reduce code duplication.
👍 refactored the tests by creating a shared/parametrized tests suite and passing just client/server specific functions and hooks - so each generic MCP test is created only once. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces additional unit and integration tests for the project and improves test configuration and helper functions. It also adds a new help tool to the MCP server and performs minor import path adjustments.
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Updates test file inclusion pattern and adds a test timeout setting. |
| tests/unit/tools.utils.test.ts | Adds comprehensive unit tests for utility functions. |
| tests/unit/tools.actor.test.ts | Adjusts import paths for consistency. |
| tests/unit/mcp.utils.test.ts | Updates test case naming and import paths. |
| tests/unit/input.test.ts | Adjusts import paths for consistency. |
| tests/integration/suite.ts | Implements a generic integration tests suite. |
| tests/integration/stdio.test.ts | Adds integration tests for STDIO-based client. |
| tests/integration/actor.server-streamable.test.ts | Adds integration test for server-streamable HTTP client functionality. |
| tests/integration/actor.server-sse.test.ts | Adds integration test for SSE client functionality. |
| tests/helpers.ts | Introduces helper functions to create various MCP client transports. |
| tests/actor-server-test.ts | Removed in favor of newer integration tests in other files. |
| tests/README.md | Provides documentation on running both unit and integration tests. |
| src/tools/helpers.ts | Adds a help tool with detailed usage information. |
| src/mcp/server.ts | Introduces a reset method and a close method along with integrating helpTool. |
| src/const.ts | Updates default tool configurations to include the help tool. |
Files not reviewed (1)
- package.json: Language not supported
2 participants
closes #57
closes #97
by default for
npm run testonly unit tests are executed as they do not require Apify API tokenFeatures: