ci(fips): temporarily disable ui tests - #41477
Conversation
|
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 CI workflow now runs ChangesFIPS UI CI gating
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 992-994: Update the aggregation condition around the test-ui-fips
result so a skipped job is accepted only when the PR does not have the fips
label; when fips is present, require test-ui-fips to finish successfully and
fail for skipped or other non-success results.
🪄 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: 521a0f6b-f22a-4fe6-b948-5c160efeb38f
📒 Files selected for processing (1)
.github/workflows/ci.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: ⚙️ Variables Setup
- GitHub Check: CodeQL-Build
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-04-27T18:32:21.871Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40321
File: .github/workflows/ci.yml:137-145
Timestamp: 2026-04-27T18:32:21.871Z
Learning: In .github/workflows/ci.yml, the `diff` step under `release-versions` intentionally uses a bash `if` with `gh pr diff ... | grep -q ...; then ... fi`. For non-`pull_request` workflow triggers where `GH_PR_NUM` can be empty, the `gh` command may fail, but the surrounding bash `if` is relied on to treat that failure as the condition being false and skip the `then` block, allowing the step/job to exit cleanly. Do not add extra guards for non-PR event types unless this failure/skip behavior is intentionally changed.
Applied to files:
.github/workflows/ci.yml
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
771-772: LGTM!
| if [[ '${{ needs.test-ui-fips.result }}' != 'success' && '${{ needs.test-ui-fips.result }}' != 'skipped' ]]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not accept unexpected skipped FIPS UI jobs.
When the PR has the fips label, a skipped test-ui-fips job should fail aggregation; this condition currently accepts skipped results regardless of why the job was skipped.
Proposed fix
- if [[ '${{ needs.test-ui-fips.result }}' != 'success' && '${{ needs.test-ui-fips.result }}' != 'skipped' ]]; then
+ if [[ '${{ needs.test-ui-fips.result }}' != 'success' && '${{ contains(github.event.pull_request.labels.*.name, 'fips') }}' == 'true' ]]; then
exit 1
fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 992 - 994, Update the aggregation
condition around the test-ui-fips result so a skipped job is accepted only when
the PR does not have the fips label; when fips is present, require test-ui-fips
to finish successfully and fail for skipped or other non-success results.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fips-ddp-streamer #41477 +/- ##
=====================================================
+ Coverage 68.57% 68.59% +0.02%
=====================================================
Files 4134 4134
Lines 160807 160807
Branches 29202 29248 +46
=====================================================
+ Hits 110272 110310 +38
+ Misses 45423 45385 -38
Partials 5112 5112
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit