Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/workflows/sonar.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,28 @@ jobs:
run: npm ci --ignore-scripts

- name: Run Jest coverage
id: run-jest-coverage
continue-on-error: true
run: npm run test:unit:coverage

- name: Verify Jest LCOV
id: verify-jest-lcov
if: always()
run: test -s coverage/jest/lcov.info

- name: Upload Jest coverage report
if: always()
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
with:
name: jest-coverage-${{ github.run_id }}
path: coverage/jest
if-no-files-found: error
retention-days: 14

- name: Fail Jest coverage job when tests or report failed
if: always() && (steps.run-jest-coverage.outcome != 'success' || steps.verify-jest-lcov.outcome != 'success')
run: exit 1

Comment on lines 64 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove if: always() to prevent steps from running on cancellation or setup failure.

Using if: always() forces these steps to run even if the workflow is cancelled or if critical setup steps (like checkout or npm ci) fail, leading to confusing cascading errors in CI logs and wasted runner time. Additionally, checking outcome != 'success' in the final step will falsely trigger a job failure if the test step was skipped (e.g., due to a failed setup).

Since the coverage execution steps use continue-on-error: true, their failure does not mark the job as failed. Thus, subsequent steps will naturally continue running without needing if: always().

By applying continue-on-error: true to the intermediate verification/render steps and explicitly checking for outcome == 'failure' (instead of != 'success') in the final failure step, we achieve the exact desired behavior safely:

  1. Coverage generation, verification, and upload continue if tests fail.
  2. They are properly skipped if setup steps fail or the workflow is cancelled.
  3. The final step correctly fails the job without triggering incorrectly on skipped steps.
  • .github/workflows/sonar.yml#L64-L81: Remove if: always(), add continue-on-error: true to the verification step, and update the final step condition.
  • .github/workflows/sonar.yml#L110-L131: Remove if: always(), add continue-on-error: true to the render/verification steps, and update the final step condition.
  • .github/workflows/sonar.yml#L162-L179: Remove if: always(), add continue-on-error: true to the verification step, and update the final step condition.
🛠️ Proposed fixes

For unit-coverage:

       - name: Run Jest coverage
         id: run-jest-coverage
         continue-on-error: true
         run: npm run test:unit:coverage
 
       - name: Verify Jest LCOV
         id: verify-jest-lcov
-        if: always()
+        continue-on-error: true
         run: test -s coverage/jest/lcov.info
 
       - name: Upload Jest coverage report
-        if: always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
         with:
           name: jest-coverage-${{ github.run_id }}
           path: coverage/jest
           if-no-files-found: error
           retention-days: 14
 
       - name: Fail Jest coverage job when tests or report failed
-        if: always() && (steps.run-jest-coverage.outcome != 'success' || steps.verify-jest-lcov.outcome != 'success')
+        if: steps.run-jest-coverage.outcome == 'failure' || steps.verify-jest-lcov.outcome == 'failure'
         run: exit 1

For component-coverage:

       - name: Render Cypress component report
-        if: always()
+        id: render-cypress-report
+        continue-on-error: true
         run: npm run coverage:component
 
       - name: Verify Cypress LCOV
         id: verify-cypress-lcov
-        if: always()
+        continue-on-error: true
         run: test -s coverage/component/lcov.info
 
       - name: Upload Cypress component coverage report
-        if: always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
         with:
           name: component-coverage-${{ github.run_id }}
           path: coverage/component
           if-no-files-found: error
           retention-days: 14
 
       - name: Fail Cypress coverage job when tests or report failed
-        if: always() && (steps.run-cypress-coverage.outcome != 'success' || steps.verify-cypress-lcov.outcome != 'success')
+        if: steps.run-cypress-coverage.outcome == 'failure' || steps.render-cypress-report.outcome == 'failure' || steps.verify-cypress-lcov.outcome == 'failure'
         run: exit 1

For mobile-coverage:

       - name: Verify mobile Jest LCOV
         id: verify-mobile-lcov
-        if: always()
+        continue-on-error: true
         run: test -s ui/mobile/coverage/lcov.info
 
       - name: Upload mobile Jest coverage report
-        if: always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
         with:
           name: mobile-coverage-${{ github.run_id }}
           path: ui/mobile/coverage
           if-no-files-found: error
           retention-days: 14
 
       - name: Fail mobile coverage job when tests or report failed
-        if: always() && (steps.run-mobile-coverage.outcome != 'success' || steps.verify-mobile-lcov.outcome != 'success')
+        if: steps.run-mobile-coverage.outcome == 'failure' || steps.verify-mobile-lcov.outcome == 'failure'
         run: exit 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Verify Jest LCOV
id: verify-jest-lcov
if: always()
run: test -s coverage/jest/lcov.info
- name: Upload Jest coverage report
if: always()
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
with:
name: jest-coverage-${{ github.run_id }}
path: coverage/jest
if-no-files-found: error
retention-days: 14
- name: Fail Jest coverage job when tests or report failed
if: always() && (steps.run-jest-coverage.outcome != 'success' || steps.verify-jest-lcov.outcome != 'success')
run: exit 1
- name: Verify Jest LCOV
id: verify-jest-lcov
continue-on-error: true
run: test -s coverage/jest/lcov.info
- name: Upload Jest coverage report
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
with:
name: jest-coverage-${{ github.run_id }}
path: coverage/jest
if-no-files-found: error
retention-days: 14
- name: Fail Jest coverage job when tests or report failed
if: steps.run-jest-coverage.outcome == 'failure' || steps.verify-jest-lcov.outcome == 'failure'
run: exit 1
📍 Affects 1 file
  • .github/workflows/sonar.yml#L64-L81 (this comment)
  • .github/workflows/sonar.yml#L110-L131
  • .github/workflows/sonar.yml#L162-L179
🤖 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/sonar.yml around lines 64 - 81, Remove if: always() from
the coverage verification, upload, and final failure steps in
.github/workflows/sonar.yml at ranges 64-81, 110-131, and 162-179. Add
continue-on-error: true to each requested verification/render step, and update
each final coverage failure condition to check only for outcome == 'failure' so
skipped steps do not fail the job while actual failures still do.

component-coverage:
name: Cypress component coverage
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
Expand All @@ -94,22 +103,32 @@ jobs:
run: npx --no-install cypress install

- name: Run Cypress component coverage
id: run-cypress-coverage
continue-on-error: true
run: npm run test:component:coverage

- name: Render Cypress component report
if: always()
run: npm run coverage:component

- name: Verify Cypress LCOV
id: verify-cypress-lcov
if: always()
run: test -s coverage/component/lcov.info

- name: Upload Cypress component coverage report
if: always()
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
with:
name: component-coverage-${{ github.run_id }}
path: coverage/component
if-no-files-found: error
retention-days: 14

- name: Fail Cypress coverage job when tests or report failed
if: always() && (steps.run-cypress-coverage.outcome != 'success' || steps.verify-cypress-lcov.outcome != 'success')
run: exit 1

mobile-coverage:
name: Mobile Jest coverage
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
Expand All @@ -136,19 +155,28 @@ jobs:
run: npm --prefix ui/mobile ci --ignore-scripts

- name: Run mobile Jest coverage
id: run-mobile-coverage
continue-on-error: true
run: npm --prefix ui/mobile run test:coverage -- --ci

- name: Verify mobile Jest LCOV
id: verify-mobile-lcov
if: always()
run: test -s ui/mobile/coverage/lcov.info

- name: Upload mobile Jest coverage report
if: always()
uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6
with:
name: mobile-coverage-${{ github.run_id }}
path: ui/mobile/coverage
if-no-files-found: error
retention-days: 14

- name: Fail mobile coverage job when tests or report failed
if: always() && (steps.run-mobile-coverage.outcome != 'success' || steps.verify-mobile-lcov.outcome != 'success')
run: exit 1

sonarqube-main:
name: SonarQube main analysis
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
Expand Down