Skip to content

test: adding tests for GitHub official remote server #300

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

didier-durand
Copy link
Contributor

Description

GitHub has recently announced (June 12th) the availability of its official remote MCP server: https://github.blog/changelog/2025-06-12-remote-github-mcp-server-is-now-available-in-public-preview/

Tech details at https://github.com/github/github-mcp-server/blob/main/docs/remote-server.md

This GitHub server is available at https://api.githubcopilot.com/mcp/

Lots of use cases dealing with source code in Strands Agents will leverage this GitHub feature.

So, we believe that it's good - given the fact that GitHub server is permanently available - to include some tests in Strands's test harness to ensure that the integration keeps working.

This PR only contains 2 tests (list_tools and list commits) for now. But, we can augment it with tests for additional verbs if deemed useful.

Related Issues

N/A

Documentation PR

For these tests to run, a GitHub secret will have to be added on the repo. It has to contain the GitHub PAT allowing access to the chosen repo (this repo itself in this case).

The creation of this token is described at https://api.githubcopilot.com/mcp/

It used in the PR code this way: "headers": {HEADER_AUTHORIZATION: f"Bearer {MCP_GITHUB_PAT}"}

The GitHub workflow of the Strands Repo will have to be updated to supply this token to the build / test job as an env var.

I am happy to (just tell me):

  1. update the workflow in a subsequent PR if you prefer
  2. create a doc PR in official Strands doc to allow users to also leverage the MCP server easily.

Type of Change

New feature (by GitHub)

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • [X ] I ran hatch run prepare

See attached pytest.log when running on my machine

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

pytest.log

Copy link
Member

@awsarron awsarron left a comment

Choose a reason for hiding this comment

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

Thanks for this one @didier-durand.

This seems to be more appropriate as an integration test in https://github.com/strands-agents/sdk-python/tree/main/tests-integ instead of a unit test. Could we move the test case in to https://github.com/strands-agents/sdk-python/blob/main/tests-integ/test_mcp_client.py ?

@didier-durand
Copy link
Contributor Author

Hi @awsarron,
I agree that it can be moved tot he directory for integ tests.
But, I think that I should remain in its own separate script rather then be merged with integ tests of local MCP servers. I believe that we should isolate the tests for official remote servers in isolated script due to their specific requirements. I am currently working on some for HubSpot, Square UP: they all have slightly different requirements which will be hard to combine in a single integ test script.

@awsarron
Copy link
Member

awsarron commented Jul 2, 2025

Hi @awsarron, I agree that it can be moved tot he directory for integ tests. But, I think that I should remain in its own separate script rather then be merged with integ tests of local MCP servers. I believe that we should isolate the tests for official remote servers in isolated script due to their specific requirements. I am currently working on some for HubSpot, Square UP: they all have slightly different requirements which will be hard to combine in a single integ test script.

Sounds good to me!

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