Skip to content

refactor: remove the unnecessary tool and parameter, implement unit tests#39

Merged
wu-sheng merged 10 commits intoapache:mainfrom
Fine0830:test/mcp
Mar 31, 2026
Merged

refactor: remove the unnecessary tool and parameter, implement unit tests#39
wu-sheng merged 10 commits intoapache:mainfrom
Fine0830:test/mcp

Conversation

@Fine0830
Copy link
Copy Markdown
Member

@Fine0830 Fine0830 commented Mar 30, 2026

  • Remove the set_skywalking_url tool and SW-URL parameter.
  • Validate properties for tools.
  • Implement unit tests.

Signed-off-by: Qiuxia Fan qiuxiafan@apache.org

@Fine0830 Fine0830 added this to the 0.2.0 milestone Mar 30, 2026
@Fine0830 Fine0830 added bug Something isn't working test labels Mar 30, 2026
Copy link
Copy Markdown

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

Refactors network transports to stop accepting the SW-URL request header (SSE/HTTP now always use the configured --sw-url/default), and introduces unit tests + CI coverage to lock in the intended URL/auth/session behavior.

Changes:

  • Removed SW-URL header-based URL resolution for SSE/HTTP contexts; always use configuredSkyWalkingURL().
  • Added unit tests covering URL finalization, ${ENV_VAR} credential resolution, session override behavior, and server registry expectations.
  • Added make test / make test-cover targets and wired make test into the CI workflow; updated docs to match the new transport behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/swmcp/server.go Removes header-based URL selection and updates SSE/HTTP context enrichment to use CLI-configured URL/auth only.
internal/swmcp/session.go Updates the session tool help text to remove references to SW-URL headers.
internal/swmcp/server_test.go Adds tests for configured URL, env var resolution, configured auth injection, and session override semantics.
internal/swmcp/server_registry_test.go Adds tests asserting expected tool/prompt/resource registration (currently via reflect/unsafe).
README.md Documents updated URL behavior across stdio, sse, and streamable transports.
Makefile Adds test and test-cover targets and exposes flags/package selectors.
CLAUDE.md Updates documentation around transport URL priority and existence of unit tests.
.github/workflows/CI.yaml Runs make test as part of the CI build job.

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

@Fine0830 Fine0830 changed the title refactor: remove the SW-URL from the HTTP header and add unit tests refactor: remove the unnecessary tool and parameter, implement unit tests Mar 31, 2026
@wu-sheng wu-sheng merged commit 74320e3 into apache:main Mar 31, 2026
3 checks passed
@Fine0830 Fine0830 deleted the test/mcp branch March 31, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants