Skip to content

Conversation

@cardoso
Copy link
Member

@cardoso cardoso commented Nov 24, 2025

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Bug Fixes
    • JIRA integration now correctly skips reporting for tests marked as expected to fail, reducing unnecessary notifications and keeping your issue tracker cleaner.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings November 24, 2025 20:25
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 24, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2025

⚠️ No Changeset found

Latest commit: d71a87f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 24, 2025

Walkthrough

The change adds an early return condition to JIRAReporter.onTestEnd to skip processing test reports when a test has expectedStatus of 'failed', preventing JIRA payloads from being sent for expected failures.

Changes

Cohort / File(s) Summary
JIRA Reporter Logic
apps/meteor/reporters/jira.ts
Added early return in onTestEnd to skip processing when test.expectedStatus equals 'failed'

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Straightforward conditional check addition
  • Single line modification with clear intent
  • No new complexity or logic branches introduced

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Poem

🐰 A test that's expected to fail,
No need for JIRA's tale,
Skip it early, quick and clean,
The wisest logic I have seen!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly matches the main change: adding handling for expected test failures in the JIRA reporter by skipping payload processing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-7.10.5-37595

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 326562e and d71a87f.

📒 Files selected for processing (1)
  • apps/meteor/reporters/jira.ts (1 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
🧠 Learnings (1)
📚 Learning: 2025-11-24T17:08:17.039Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.039Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts

Applied to files:

  • apps/meteor/reporters/jira.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: CodeQL analysis (javascript-typescript)
  • GitHub Check: Builds matrix rust bindings against alpine
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/reporters/jira.ts (1)

58-60: LGTM! Correctly prevents JIRA tickets for expected failures.

The early return for test.expectedStatus === 'failed' properly filters out tests marked to fail (e.g., using Playwright's .fail() annotation), ensuring only unexpected failures create JIRA tickets for flaky test tracking.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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 enhances the JIRA reporter to skip reporting tests that are expected to fail, preventing false positives in JIRA issue tracking.

Key Changes

  • Added a check to filter out tests with expectedStatus === 'failed' before creating JIRA issues

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.04%. Comparing base (326562e) to head (d71a87f).
⚠️ Report is 1 commits behind head on release-7.10.5.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           release-7.10.5   #37601      +/-   ##
==================================================
- Coverage           65.06%   65.04%   -0.03%     
==================================================
  Files                3255     3255              
  Lines              109627   109627              
  Branches            20721    20717       -4     
==================================================
- Hits                71331    71309      -22     
- Misses              35769    35790      +21     
- Partials             2527     2528       +1     
Flag Coverage Δ
unit 71.51% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@scuciatto scuciatto self-assigned this Nov 25, 2025
@scuciatto scuciatto added stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge labels Nov 25, 2025
@kodiakhq kodiakhq bot merged commit 4510a99 into release-7.10.5 Nov 25, 2025
61 of 65 checks passed
@kodiakhq kodiakhq bot deleted the backport-7.10.5-37595 branch November 25, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants