- Notifications
You must be signed in to change notification settings - Fork897
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this 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
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this 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.
fa0e36c intomainUh oh!
There was an error while loading.Please reload this page.
add initial tests
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:
We have added a few simple test helper functions to try to simplify the tests a bit.