Fix github team checking for pagination#669
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis pull request bumps the package version to 4.9.3 and modifies the GitHub team lookup functionality. The team existence check in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/unit/functions/github.test.ts (1)
64-84: Add a cross-page existence test to lock in the pagination fix.Current “team already exists” test only covers a single page. Please add a case where page 1 has no match and page 2 has the target team.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/functions/github.test.ts` around lines 64 - 84, Add a new unit test for createGithubTeam that verifies pagination: mock mockOctokit.paginate.iterator (used in the existing test) to return an async iterator representing two pages where the first page's data array does not contain "Test Team" and the second page does contain it (use createAsyncIterator to build the sequence), then assert the function returns { updated: false, id: <existingTeamId> }, that mockLogger.info was called with the same "Team \"Test Team\" already exists with id: ..." message, and that mockOctokit.request was not called; mirror assertions from the single-page test but use the two-page iterator to lock in the pagination fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/functions/github.test.ts`:
- Line 74: Prettier is failing due to missing trailing commas in several
array/object literals (you can spot them where lines end with "])"); update each
affected test array/object so the last element has a trailing comma before the
closing bracket/paren (e.g., change "])" to " ,])" or add a trailing comma
inside object literals), then run prettier --write (or your repo's formatter) to
reformat; search the test file for occurrences of "])" and similar closing
tokens in test data to locate and fix the instances.
- Around line 186-188: Replace the throw-only async generator expressions like
"(async function* () { throw baseError; })()" used as mocks with simple mock
implementations that throw, e.g. use mockImplementation(() => { throw baseError;
}) or mockImplementation(async () => { throw baseError; }) depending on whether
the mocked function is async; locate the offending expressions in
tests/unit/functions/github.test.ts (the occurrences of "(async function* () {
throw baseError; })()") and update both instances accordingly.
- Around line 55-57: Replace the function-expression callbacks passed to
(Octokit as any).mockImplementation and the other mockImplementation call in
tests/unit/functions/github.test.ts with arrow-function callbacks to satisfy the
prefer-arrow-callback ESLint rule; specifically, update the mockImplementation
usages that return mockOctokit (and the second occurrence returning its mock) to
use () => mockOctokit (or appropriate arrow form) so the tests keep the same
behavior but conform to the lint configuration.
---
Nitpick comments:
In `@tests/unit/functions/github.test.ts`:
- Around line 64-84: Add a new unit test for createGithubTeam that verifies
pagination: mock mockOctokit.paginate.iterator (used in the existing test) to
return an async iterator representing two pages where the first page's data
array does not contain "Test Team" and the second page does contain it (use
createAsyncIterator to build the sequence), then assert the function returns {
updated: false, id: <existingTeamId> }, that mockLogger.info was called with the
same "Team \"Test Team\" already exists with id: ..." message, and that
mockOctokit.request was not called; mirror assertions from the single-page test
but use the two-page iterator to lock in the pagination fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d887ce3-0c02-4c21-ae06-a305d57dfd1e
📒 Files selected for processing (3)
package.jsonsrc/api/functions/github.tstests/unit/functions/github.test.ts
|
1 participant
Summary by CodeRabbit