Skip to content

add initial tests #14

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

Merged
merged 1 commit into from
Mar 17, 2025
Merged

add initial tests #14

merged 1 commit into from
Mar 17, 2025

Conversation

juruen
Copy link
Collaborator

@juruen juruen commented Mar 17, 2025

Context

First of all, apologies for the large PR. I'm happy to split it into smaller PRs if needed.

As we talked off-line, we need to start adding tests to the codebase. Although black-box tests will probably be easier to read and maintain, I wanted to make sure that we have some testing in place, even if it's not the best choice yet.

This PR adds tests that are based in testing each one of the individual tools by mocking the GitHub client. We verify that:

  • The tool schema is expected
  • A successful use of the tool by checking that:
    • The expected arguments are passed to the GitHub client
    • The response is parsed and returned
    • A limited number of permutations for those those tools that have optional args
  • A failed use case of the tool

We have added a few simple test helper functions to try to simplify the tests a bit.

@Copilot Copilot AI review requested due to automatic review settings March 17, 2025 12:18
Copy link
Contributor

@Copilot 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 introduces initial tests for various GitHub tools and updates HTTP status code checks for consistency.

  • Added tests for GitHub issues, code scanning, pull requests, and server functionality.
  • Updated conditional checks to use http.Status* constants in place of literal numeric values.
  • Established helper functions for MCP request creation and text result extraction.

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
pkg/github/issues_test.go Introduces tests for issue retrieval and comment creation.
pkg/github/code_scanning_test.go Adds tests for fetching code scanning alerts.
pkg/github/server_test.go Tests the authenticated user tool and accepted error detection.
pkg/github/helper_test.go Provides helper functions used across tests.
pkg/github/server.go Replaces hardcoded status codes with http.Status* constants.
pkg/github/pullrequests.go Applies consistent status code constants and adds accepted error check.
pkg/github/repositories.go Updates status code constants; potential issue in fork repository handling.
pkg/github/issues.go Replaces numeric status codes with http.Status* constants.
pkg/github/code_scanning.go Uses http.Status* constants for response validation.
Files not reviewed (1)
  • go.mod: Language not supported

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@juruen juruen force-pushed the juruen/add-tests branch from ff5e309 to 3dddc2a Compare March 17, 2025 12:26
Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums left a comment

Choose a reason for hiding this comment

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

To me it feels ok to have a big PR to introduce the tests. I mean stacked PRs is a possibility, but I am happy to just make rapid progress, and then be more strict once we have an MVP.

@juruen juruen merged commit fa0e36c into main Mar 17, 2025
3 checks passed
@juruen juruen deleted the juruen/add-tests branch March 17, 2025 17:45
manian0430 pushed a commit to ChrisLally/github-mcp-server that referenced this pull request Apr 12, 2025
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