Refactor to use rest.search.issuesAndPullRequests #18
Conversation
klaasnicolaas
added
the
refactor
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 102 95 -7
Branches 26 15 -11
=========================================
- Hits 102 95 -7 ☔ View full report in Codecov by Sentry. |
WalkthroughWalkthroughThe recent updates enhance testing for fetching and locking GitHub issues and pull requests, integrating rate limit handling and error management. It includes mock data adjustments, new test cases for activity-based issue locking, and revised function parameters. The changes also refactor imports, mock setups, and test case structures to improve robustness and coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant Test Suite
participant GitHub Mock
participant Core Mock
Test Suite->>GitHub Mock: Fetch issues and PRs
GitHub Mock-->>Test Suite: Return fetched items
Test Suite->>Core Mock: Process items
Core Mock-->>Test Suite: Return processed results
Test Suite->>GitHub Mock: Lock stale items
GitHub Mock-->>Test Suite: Confirm lock
Note right of Test Suite: New test cases simulate activity-based locking
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/index.ts (1)
Line range hint
16-97:
LGTM! But consider removing the unnecessary else clause.The function is well-written but includes an unnecessary else clause as previous branches break early.
- } else { - core.warning('Initial rate limit too low, stopping processing.') - } + core.warning('Initial rate limit too low, stopping processing.')
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
Files selected for processing (6)
- tests/fetch_lock.test.ts (1 hunks)
- tests/issues.test.ts (4 hunks)
- tests/pull_requests.test.ts (4 hunks)
- tests/ratelimit.test.ts (2 hunks)
- tests/run.test.ts (4 hunks)
- src/index.ts (3 hunks)
Files not reviewed due to errors (1)
- tests/pull_requests.test.ts (no review received)
Additional context used
Biome
src/index.ts
[error] 152-155: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (19)
__tests__/run.test.ts (3)
3-3: Import statement looks good.The import of
runfromsrc/indexis correct and necessary for the test.
14-18: Mock date setup looks good.Setting a mock date using
jest.useFakeTimers().setSystemTime(currentDate)is a good practice for consistent test results.
32-34: Mock Octokit setup looks good.The
search.issuesAndPullRequestsmethod is correctly mocked in themockOctokitobject.__tests__/ratelimit.test.ts (4)
30-32: Mock Octokit setup looks good.The
search.issuesAndPullRequestsmethod is correctly mocked in themockOctokitobject.
Line range hint
3-3:
Import statements look good.The import of
processPullRequestsandfetchIssuesAndPRsfromsrc/indexis correct and necessary for the test.
Line range hint
14-18:
Mock date setup looks good.Setting a mock date using
jest.useFakeTimers().setSystemTime(currentDate)is a good practice for consistent test results.
32-34: Mock Octokit setup looks good.The
search.issuesAndPullRequestsmethod is correctly mocked in themockOctokitobject.__tests__/issues.test.ts (3)
3-3: Import statements look good.The import of
processIssuesandfetchIssuesAndPRsfromsrc/indexis correct and necessary for the test.
14-18: Mock date setup looks good.Setting a mock date using
jest.useFakeTimers().setSystemTime(currentDate)is a good practice for consistent test results.
32-34: Mock Octokit setup looks good.The
search.issuesAndPullRequestsmethod is correctly mocked in themockOctokitobject.__tests__/fetch_lock.test.ts (5)
62-78: LGTM! Test case for rate limit warning is well-written.The test case correctly mocks the rate limit response and verifies the warning.
80-87: LGTM! Test case for fetching issues and PRs is well-written.The test case correctly mocks the API call and verifies the parameters.
89-117: LGTM! Test case for fetching multiple pages of issues and PRs is well-written.The test case correctly mocks the API calls and verifies the results.
119-136: LGTM! Test case for handling errors during fetching is well-written.The test case correctly mocks the error response and verifies the error handling.
138-155: LGTM! Test case for handling errors during locking is well-written.The test case correctly mocks the error response and verifies the error handling.
src/index.ts (4)
98-160: LGTM! FunctionfetchIssuesAndPRsis well-written.The function includes proper error handling and correctly fetches issues and PRs.
Tools
Biome
[error] 152-155: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
164-212: LGTM! FunctionprocessIssuesis well-written.The function includes proper logging and error handling.
214-256: LGTM! FunctionprocessPullRequestsis well-written.The function includes proper logging and error handling.
258-294: LGTM! FunctionlockItemis well-written.The function includes proper error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
src/index.ts (1)
Line range hint
16-95:
Therunfunction is well-structured but can be optimized.The function correctly initializes inputs, checks the rate limit, fetches issues and PRs, processes them, and checks the rate limit again. However, the else clause after checking the rate limit can be omitted for cleaner code.
- } - core.warning('Initial rate limit too low, stopping processing.') + } else { + core.warning('Initial rate limit too low, stopping processing.') + }Tools
Biome
[error] 151-154: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
Files selected for processing (2)
- tests/fetch_lock.test.ts (1 hunks)
- src/index.ts (2 hunks)
Additional context used
Biome
src/index.ts
[error] 151-154: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (12)
__tests__/fetch_lock.test.ts (7)
1-11: Good setup for mocking core and GitHub modules.The initial setup logic correctly mocks the necessary modules and defines mock objects for core and GitHub.
12-57: Initialization and setup for tests are well done.The
beforeEachblock correctly initializes the mock Octokit instance and sets up the test environment.
59-97: Test case for rate limit buffer is well-implemented.The test case correctly simulates a rate limit condition and verifies that the appropriate warning is issued.
99-106: Test case for fetching issues and PRs is accurate.The test case correctly verifies the fetching logic and ensures that the API call is made with the correct parameters.
108-136: Test case for fetching multiple pages of issues and PRs is thorough.The test case correctly simulates multiple pages and verifies that all items are fetched.
138-155: Test case for error handling during fetching is effective.The test case correctly simulates an error and verifies that the appropriate failure message is set.
157-174: Test case for error handling during locking is effective.The test case correctly simulates an error and verifies that the appropriate failure message is set.
src/index.ts (5)
97-159: ThefetchIssuesAndPRsfunction is well-implemented.The function correctly handles fetching closed issues and PRs, checks the rate limit, and handles pagination.
Tools
Biome
[error] 151-154: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
163-211: TheprocessIssuesfunction is well-implemented.The function correctly processes and locks inactive issues, and sets the output for locked issues.
213-255: TheprocessPullRequestsfunction is well-implemented.The function correctly processes and locks inactive pull requests, and sets the output for locked pull requests.
257-293: ThelockItemfunction is well-implemented.The function correctly handles the locking logic and error handling.
Line range hint
294-311:
ThecheckRateLimitfunction is well-implemented.The function correctly handles the rate limit checking and error handling.
Tools
Biome
[error] 151-154: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Thanks to this refactor, we can search very specifically for issues and pull requests that are closed but not yet locked.
Summary by CodeRabbit