-
Notifications
You must be signed in to change notification settings - Fork 13k
test: Update JIRA API version and handle errors in test result reporting #37723
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
Conversation
- Changed JIRA API endpoints from version 2 to 3. - Added error handling in the `onTestEnd` method to log errors when sending test results to JIRA.
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe JIRA reporter in the Meteor testing suite is upgraded to use JIRA REST API v3 endpoints instead of v2 across search, issue creation, updates, and comment operations. A new private method wraps the test-end handler in error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37723 +/- ##
===========================================
+ Coverage 67.78% 67.88% +0.10%
===========================================
Files 3449 3449
Lines 113987 113972 -15
Branches 20956 20955 -1
===========================================
+ Hits 77262 77375 +113
+ Misses 34610 34473 -137
- Partials 2115 2124 +9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/reporters/jira.ts (1)
85-103: Error-path search URL is missing the?before query paramsIn the error message construction on Line [100], the URL is built as:
`${this.url}/rest/api/3/search${new URLSearchParams({ ... })}`unlike the actual request on Line [85], which correctly includes
...?. This only affects diagnostics, but it makes the logged URL invalid and slightly harder to test/copy.You can align the two with:
- `${this.url}/rest/api/3/search${new URLSearchParams({ - jql: `project = FLAKY AND summary ~ '${payload.name}'`, - })}`, + `${this.url}/rest/api/3/search?${new URLSearchParams({ + jql: `project = FLAKY AND summary ~ '${payload.name}'`, + })}`,
🧹 Nitpick comments (2)
apps/meteor/reporters/jira.ts (2)
45-53: Error-handling wrapper aroundonTestEndis appropriate; consider logging more contextWrapping the body in
try/catchand delegating to_onTestEndavoids JIRA failures bubbling into the test run, which is exactly what you want here. You might make the log slightly more actionable by including the test identity, e.g.,test.titleortest.id, so operators can correlate rapidly when this fires.
120-155: v3 issue/comment endpoints look correct; consider checkingokon non-search callsThe migrations to
/rest/api/3/issue/${existing.key},/rest/api/3/issue/${existing.key}/comment,/rest/api/3/issue, and/rest/api/3/issue/${issue}/commentare syntactically consistent with the v3 REST API and the search change.Right now, only the search request checks
response.ok; the update/create/comment calls will silently proceed even on 4xx/5xx (and may then try to parse or use a missingkey). Given you now have a top-leveltry/catch, it would be reasonable to promote HTTP failures here into thrown errors so they get logged in the same way as other failures.For example, for the create call around Line [184]:
- const responseIssue = await fetch(`${this.url}/rest/api/3/issue`, { ... }); - - const issue = (await responseIssue.json()).key; + const responseIssue = await fetch(`${this.url}/rest/api/3/issue`, { ... }); + if (!responseIssue.ok) { + throw new Error(`JIRA: Failed to create issue: ${responseIssue.status} ${responseIssue.statusText}`); + } + const issue = (await responseIssue.json()).key;(and similarly for the update/comment requests if you want consistent behavior).
Please verify against your JIRA instance (or its REST API docs) that these v3 endpoints and error-handling expectations match your deployment, and that the reporter still behaves as intended when JIRA returns non‑2xx responses.
Also applies to: 184-214
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/reporters/jira.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/reporters/jira.ts
onTestEndmethod to log errors when sending test results to JIRA.Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.