Skip to content

feat: add github workflow for test coverage #1034

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.

Sign up for GitHub

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

Open
tejaskh3 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

feat: add github workflow for test coverage #1034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
955ac75
feat: add github workflow for test coverage
tejaskh3 Apr 18, 2025
8a4b7ff
feat: add ember-cli-code-coverage
tejaskh3 Apr 18, 2025
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
38 changes: 38 additions & 0 deletions .github/workflows/coverage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: Test Coverage

on:
pull_request:
branches: [ main, master, develop ]
Comment on lines +3 to +5
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

YAML syntax issues in workflow trigger configuration.

There are spacing issues in the bracket notation of the branches array.

 on:
   pull_request:
-    branches: [ main, master, develop ]
+    branches: [main, master, develop]
📝 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
on:
pull_request:
branches: [ main, master, develop ]
on:
pull_request:
branches: [main, master, develop]
🧰 Tools
🪛 YAMLlint (1.35.1)

[warning] 3-3: truthy value should be one of [false, true]

(truthy)

[error] 5-5: too many spaces inside brackets

(brackets)

[error] 5-5: too many spaces inside brackets

(brackets)


jobs:
coverage:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v3
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Update GitHub Actions checkout to latest version.

The runner for the checkout action is outdated.

-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
📝 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
- uses: actions/checkout@v3
- uses: actions/checkout@v4
🧰 Tools
🪛 actionlint (1.7.4)

14-14: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


- name: Install pnpm
uses: pnpm/action-setup@v2
with:
version: 8
run_install: false

- name: Setup Node.js
uses: actions/setup-node@v3
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Update GitHub Actions setup-node to latest version.

The runner for the setup-node action is outdated.

-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
📝 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
uses: actions/setup-node@v3
- uses: actions/setup-node@v4
🧰 Tools
🪛 actionlint (1.7.4)

23-23: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

with:
node-version: 18
cache: 'pnpm'

- name: Install dependencies
run: pnpm install
Comment on lines +28 to +29
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider caching node_modules for faster workflows.

While pnpm caching is configured, you could further optimize by caching node_modules.

      - name: Install dependencies
        run: pnpm install
+        
+      - name: Cache node_modules
+        uses: actions/cache@v4
+        with:
+          path: node_modules
+          key: ${{ runner.os }}-node-modules-${{ hashFiles('**/pnpm-lock.yaml') }}
+          restore-keys: |
+            ${{ runner.os }}-node-modules-
📝 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: Install dependencies
run: pnpm install
- name: Install dependencies
run: pnpm install
- name: Cache node_modules
uses: actions/cache@v4
with:
path: node_modules
key: ${{ runner.os }}-node-modules-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-node-modules-


- name: Run tests with coverage
run: pnpm run test:coverage || true
Comment on lines +31 to +32
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider failing the workflow on low coverage.

The current implementation runs tests with coverage and continues even if tests fail (|| true). While this ensures the workflow completes and reports coverage, consider adding a step to enforce a minimum coverage threshold.

You could add a step after posting the coverage comment to check the coverage percentage and fail if it's below a threshold:

- name: Check coverage threshold
  run: |
    COVERAGE=$(cat ./coverage/lcov.info | grep -oP 'LF:\K[0-9]+' | awk '{covered+=$1} END {print covered}')
    TOTAL=$(cat ./coverage/lcov.info | grep -oP 'LH:\K[0-9]+' | awk '{total+=$1} END {print total}')
    PERCENTAGE=$(awk "BEGIN { print ($TOTAL/$COVERAGE) * 100 }")
    echo "Current coverage: $PERCENTAGE%"
    if (( $(echo "$PERCENTAGE < 70" | bc -l) )); then
      echo "Coverage below threshold of 70%"
      exit 1
    fi

Copy link

@graphite-app graphite-app bot May 29, 2025

Choose a reason for hiding this comment

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

The || true suffix will make this step always exit with a success code, even when tests fail. This could hide test failures in the workflow. Consider either:

  1. Removing || true to ensure test failures are properly reported
  2. Using GitHub's continue-on-error: true at the step level for better error handling while still allowing the coverage reporting to proceed

This approach would maintain test integrity while still achieving the coverage reporting goal.

Suggested change
run: pnpm run test:coverage || true
run: pnpm run test:coverage
continue-on-error: true

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.


- name: Post coverage comment
uses: romeovs/[email protected]
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
lcov-file: ./coverage/lcov.info
Comment on lines +35 to +38
Copy link

@coderabbitai coderabbitai bot Apr 18, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider adding a badge to README.

The coverage report is posted as a comment, but you might want to also add a coverage badge to your README for visibility.

After this workflow is established, you could add the following to your README.md:

[![Test Coverage](https://img.shields.io/codecov/c/github/Real-Dev-Squad/website-www/main.svg)](https://codecov.io/gh/Real-Dev-Squad/website-www)

This requires setting up integration with a service like Codecov, which could be a future enhancement.

7 changes: 7 additions & 0 deletions ember-cli-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ module.exports = function (defaults) {
'ember-fetch': {
preferNative: true,
},
babel: {
plugins: [
...require('ember-cli-code-coverage').buildBabelPlugin({
embroider: true,
}),
],
},
});

// Use `app.import` to add additional libraries to the generated
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
"cropperjs": "^1.5.12",
"d3-cloud": "1.2.7",
"dotenv": "16.0.2",
"ember-cli-qrcode": "^2.1.0",
"ember-cli-fastboot": "4.1.5",
"ember-cli-qrcode": "^2.1.0",
"ember-d3": "0.5.1",
"ember-phone-input": "^10.0.0",
"exists-sync": "0.1.0",
Expand Down Expand Up @@ -66,6 +66,7 @@
"ember-cli-babel": "8.2.0",
"ember-cli-clean-css": "3.0.0",
"ember-cli-clipboard": "1.0.0",
"ember-cli-code-coverage": "^3.1.0",
"ember-cli-dependency-checker": "3.3.2",
"ember-cli-htmlbars": "6.3.0",
"ember-cli-inject-live-reload": "2.1.0",
Expand Down
154 changes: 154 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions tests/test-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,19 @@ import * as QUnit from 'qunit';
import { setApplication } from '@ember/test-helpers';
import { setup } from 'qunit-dom';
import { start } from 'ember-qunit';
import {
forceModulesToBeLoaded,
sendCoverage,
} from 'ember-cli-code-coverage/test-support';

setApplication(Application.create(config.APP));

setup(QUnit.assert);
setupSinon();

QUnit.done(async function () {
forceModulesToBeLoaded();
await sendCoverage();
});

start();
Loading