Add [Coderabbit] PR Stats service and tests #10749
Conversation
- Implemented a new service `CoderabbitStats` to fetch and display pull request statistics from the CodeRabbit API. - Created a corresponding tester file to validate the service's functionality, including tests for valid repositories, repository not found, and server errors. - The service returns a badge with the number of PRs and appropriate error messages based on the API response. This addition enhances the analysis capabilities of the application by integrating CodeRabbit statistics.
|
chris48s
changed the title
There was a problem hiding this comment.
Thank you for the contribution.
I noticed you did not add any live tests (making real API calls to external service) and only mocked responses.
Please give a quick read to this section of our docs about tests, as mentioned its best to include at least one live test so we know our service is still live and up to date with upstream.
Note that when we call our badge, we are allowing it to communicate with an external service without mocking the response. We write tests which interact with external services, which is unusual practice in unit testing. We do this because one of the purposes of service tests is to notify us if a badge has broken due to an upstream API change. For this reason it is important for at least one test to call the live API without mocking the interaction
Is there a specific reason to avoid using live tests? If no could you add at least one live test, Its common for our project to have all calls to live services in tests unless we test internal functionality like data transformation of API response.
- Updated the service to fetch and display the number of reviews from the CodeRabbit API, changing the schema and badge labels accordingly. - Modified the tester file to reflect the new endpoint and expected responses, including regex for message validation. - Enhanced error handling in the service to return more descriptive error messages for invalid repositories and server errors. This change improves the accuracy of the statistics provided by the service, aligning it with the intended functionality of tracking reviews.
|
@jNullj I have made relevant changes as per your request, requesting you to see if it fits the guidelines. |
| label: 'CodeRabbit', | ||
| labelColor: '171717', | ||
| } | ||
|
|
||
| static render({ reviews }) { | ||
| return { | ||
| message: `${reviews} Reviews`, |
There was a problem hiding this comment.
From
https://github.com/badges/shields/blob/master/CONTRIBUTING.md#badge-guidelines
The left-hand side of a badge should not advertise. It should be a lowercase noun succinctly describing the meaning of the right-hand side.
So rather than CodeRabbit | 7 Reviews, this badge should be either reviews | 7 or coderabbit reviews | 7
There was a problem hiding this comment.
I have updated the text accordingly.
There was a problem hiding this comment.
The label text should be lowercase, even where we're including a service name
| color: 'ff570a', | ||
| } | ||
| } | ||
|
|
||
| static renderError({ message }) { | ||
| return { | ||
| message, | ||
| color: '9f9f9f', |
There was a problem hiding this comment.
Can we ditch the custom colours here and stick with the standard palette please.
We can just get rid of the labelColor in defaultBadgeData. All badges should use the default #555555 for the label (left side of the badge)
For the message (right side), lets go with blue. We use this for informational badges in our palette, like this one where all badges get the same colour regardless of the number.
There was a problem hiding this comment.
Can I do orange, which is one of the named colors? #fe7d37
https://github.com/badges/shields/tree/master/badge-maker#colors
There was a problem hiding this comment.
I have removed colors fully
| static category = 'analysis' | ||
| static route = { | ||
| base: 'coderabbit', | ||
| pattern: 'stats/:provider/:org/:repo', |
There was a problem hiding this comment.
Can we make the "noun" part of the route /prs or /pull-requests here. /stats doesn't really describe the information on the badge. It also leaves things more open if we ever wanted to add more CodeRabbit badges.
Docs on badge URLs: https://github.com/badges/shields/blob/master/doc/badge-urls.md
There was a problem hiding this comment.
@chris48s this would be a major change on the endpoint side as we need to change the backend code. We are on a freeze now and can make this change in Jan 2025.
There was a problem hiding this comment.
What we are declaring here is the URL for the badge on shields.io
The route on the upstream API (i.e: https://api.coderabbit.ai/stats/... ) is independent of this.
- Updated the CodeRabbitStats service to include OpenAPI documentation and improved error handling for repository not found scenarios. - Changed badge label from 'CodeRabbit' to 'CodeRabbit Reviews' for clarity. - Modified the tester file to reflect the new badge format and error messages, ensuring consistency with the service updates. - Adjusted regex patterns for message validation in tests. These changes improve the usability and accuracy of the CodeRabbit statistics service.
|
@chris48s I'm contributing a badge for the first time to shields.io. Sorry for the trouble with obvious mistakes. I've read the contributor.md and tutorial.md, but I still missed a lot. Please re-review and let me know if things are out of place. |
| get: { | ||
| summary: 'CodeRabbit Pull Request Reviews', | ||
| description: | ||
| 'By default, this badge pulls the number of PRs reviewed by [CodeRabbit](https://coderabbit.ai), AI code review tool', |
There was a problem hiding this comment.
By default
Is there an option to pull something else here?
| message: '101', | ||
| }) | ||
|
|
||
| t.create('CodeRabbitStats repo not found') |
There was a problem hiding this comment.
Could we use live examples for all types of responses?
Co-authored-by: jNullj <[email protected]>
CoderabbitStatsto fetch and display pull request statistics from the CodeRabbit API.This addition enhances the analysis capabilities of the application by integrating CodeRabbit statistics.
GitHub Issue - #10748