Skip to content
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

feat: add coverage checks with min 80% threshold #70

Merged
merged 2 commits into from
Nov 28, 2024
Merged

Conversation

ttypic
Copy link
Collaborator

@ttypic ttypic commented Nov 28, 2024

Added coverage report for SDK

Summary by CodeRabbit

  • New Features

    • Introduced a new GitHub Actions workflow for automated test coverage reporting.
    • Added Gradle setup steps to enhance the CI process.
  • Improvements

    • Integrated Kover plugin for code coverage in the Android project build configuration.
    • Expanded available plugins by adding Kover to the project.
  • Bug Fixes

    • Implemented checks to ensure minimum code coverage thresholds are met during CI.

Copy link

coderabbitai bot commented Nov 28, 2024

Walkthrough

The pull request introduces several changes to enhance the CI/CD pipeline for a Java project. A new GitHub Actions workflow file named coverage.yml is added to automate testing and code coverage reporting. Additionally, the existing workflow in check.yml is modified to include a step for setting up Gradle. The Gradle build configuration for the Android project is updated to include the Kover plugin for code coverage, along with an entry for this plugin in the libs.versions.toml file.

Changes

File Change Summary
.github/workflows/check.yml Added a new step "Set up Gradle" using gradle/gradle-build-action@v2.
.github/workflows/coverage.yml Introduced a new workflow for running tests and generating a coverage report with multiple steps.
chat-android/build.gradle.kts Added the Kover plugin with alias(libs.plugins.kover) in the Gradle build configuration.
gradle/libs.versions.toml Added Kover plugin entry with ID org.jetbrains.kotlinx.kover and version 0.9.0-RC.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant GitHubActions as GitHub Actions
    participant Gradle as Gradle
    participant CoverageReport as Coverage Report

    Developer->>GitHubActions: Push code or create PR
    GitHubActions->>Gradle: Set up environment
    GitHubActions->>Gradle: Run tests
    Gradle->>CoverageReport: Generate coverage report
    CoverageReport-->>GitHubActions: Return coverage data
    GitHubActions->>Developer: Notify coverage status
Loading

🐇 "In the land of code, where bunnies play,
New workflows hop in, brightening the day.
With Gradle set up and coverage in sight,
Our tests will now flourish, all snug and tight.
So let’s celebrate changes, both big and small,
For a codebase that's healthy, we'll cherish it all!" 🐇


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ttypic ttypic force-pushed the add-coverage-report branch from eea472a to 22ce137 Compare November 28, 2024 11:34
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (6)
.github/workflows/check.yml (1)

21-23: Consider optimizing Gradle builds with caching.

To improve CI performance, consider enabling Gradle build caching:

      - name: Set up Gradle
        uses: gradle/gradle-build-action@v2
+       with:
+         cache-read-only: ${{ github.ref != 'refs/heads/main' }}
+         gradle-home-cache-cleanup: true

Also, consider moving Java configuration to environment variables for better maintainability:

+env:
+  JAVA_VERSION: '17'
+  JAVA_DISTRIBUTION: 'temurin'
+
 jobs:
   check:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4

       - name: Set up Java 17
         uses: actions/setup-java@v4
         with:
-          distribution: 'temurin'
-          java-version: '17'
+          distribution: ${{ env.JAVA_DISTRIBUTION }}
+          java-version: ${{ env.JAVA_VERSION }}
.github/workflows/coverage.yml (3)

3-8: Consider adding branch filters for pull_request trigger

The workflow currently runs on all pull requests without branch filtering. Consider adding branch filters to optimize CI resources by only running on relevant branches.

 pull_request:
+    branches:
+      - main

35-41: Improve coverage threshold check implementation

Several improvements can be made to the coverage check:

  1. Make the threshold configurable via environment variable
  2. Use more portable arithmetic comparison
  3. Add more detailed failure message
       - name: Fail if coverage is too low
         run: |
-          MIN_COVERAGE=0.80
-          if (( $(echo "$COVERAGE < $MIN_COVERAGE" | bc -l) )); then
-            echo "Coverage ($COVERAGE) is below the minimum threshold ($MIN_COVERAGE)."
+          MIN_COVERAGE=${MIN_COVERAGE:-0.80}
+          COVERAGE_PERCENT=$(awk "BEGIN {print $COVERAGE * 100}")
+          MIN_COVERAGE_PERCENT=$(awk "BEGIN {print $MIN_COVERAGE * 100}")
+          if awk "BEGIN {exit !($COVERAGE < $MIN_COVERAGE)}"; then
+            echo "❌ Coverage: $COVERAGE_PERCENT% (Required: $MIN_COVERAGE_PERCENT%)"
+            echo "Please add more tests to increase coverage."
             exit 1
           fi
+          echo "✅ Coverage: $COVERAGE_PERCENT% (Required: $MIN_COVERAGE_PERCENT%)"

26-28: Consider enhancing coverage reporting

The current workflow could benefit from additional features:

  1. Upload coverage report as an artifact
  2. Integrate with a coverage visualization tool (e.g., Codecov)

Here's an example of how to add these enhancements:

       - name: Run tests and generate coverage report
         run: ./gradlew koverXmlReport

+      - name: Upload coverage report
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage-report
+          path: build/reports/kover/report.xml
+
+      - name: Upload to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          file: build/reports/kover/report.xml
chat-android/build.gradle.kts (2)

7-7: LGTM, but coverage configuration is missing.

While adding the Kover plugin is correct, the configuration needed to enforce the 80% coverage threshold is missing.

Add the following configuration block after the plugins section:

koverReport {
    defaults {
        xml {
            onCheck = true
        }
    }
    filters {
        excludes {
            // Add any classes that should be excluded from coverage
            classes("*BuildConfig")
        }
    }
    verify {
        rule {
            isEnabled = true
            bound {
                minValue = 80
                valueType = kotlinx.kover.api.VerificationValueType.COVERED_LINES_PERCENTAGE
            }
        }
    }
}

Also, consider using a stable version of Kover instead of 0.9.0-RC for production use. The latest stable version can be found in the Kover releases.


Line range hint 63-67: Consider adding coverage verification to the test task.

To ensure coverage checks are run as part of the testing process, you might want to link coverage verification to the test task.

Add this configuration to your test task:

tasks.withType<Test>().configureEach {
    testLogging {
        exceptionFormat = TestExceptionFormat.FULL
    }
    finalizedBy(tasks.named("koverVerify"))
}
🛑 Comments failed to post (2)
.github/workflows/coverage.yml (1)

29-34: 🛠️ Refactor suggestion

Make coverage extraction more robust

The current grep-based XML parsing is fragile and might break with different XML formats. Consider using a proper XML parsing tool.

-      - name: Extract coverage from XML
+      - name: Extract coverage from XML
         run: |
-          COVERAGE=$(grep -oP 'line-rate="([0-9.]+)"' build/reports/kover/report.xml | head -1 | grep -oP '[0-9.]+')
+          COVERAGE=$(xmlstarlet sel -t -v "//report/@line-rate" build/reports/kover/report.xml)
           echo "Coverage: ${COVERAGE}"
-          echo "coverage=${COVERAGE}" >> $GITHUB_ENV
+          echo "coverage=${COVERAGE}" >> "$GITHUB_ENV"

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 actionlint (1.7.4)

30-30: shellcheck reported issue in this script: SC2086:info:3:32: Double quote to prevent globbing and word splitting

(shellcheck)

gradle/libs.versions.toml (1)

69-69: 💡 Codebase verification

Consider using Kover 0.8.3 instead of 0.9.0-RC

The latest stable version 0.8.3 is recommended for production use. Version 0.9.0-RC is a release candidate that introduces new features but may have stability issues. Version 0.8.3 is production-ready and includes support for Android projects.

🔗 Analysis chain

Consider using a stable version of Kover.

The current RC version (0.9.0-RC) might have stability issues. Consider using the latest stable version for production use.

Let's verify the latest stable version and its compatibility with Kotlin 2.0.10:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check latest Kover versions and compatibility

# Check Maven Central for latest versions
curl -s "https://search.maven.org/solrsearch/select?q=g:org.jetbrains.kotlinx.kover+AND+a:kotlinx-kover-gradle-plugin&rows=5&wt=json" | jq -r '.response.docs[].v'

# Check GitHub releases for compatibility notes
gh api repos/Kotlin/kotlinx-kover/releases --jq '.[0:3] | .[] | {tag_name, body}'

Length of output: 3302

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (1)
.github/workflows/coverage.yml (1)

9-24: Consider adding caching for better performance

Adding caching for Gradle dependencies can significantly improve workflow execution time.

Apply this diff to enable caching:

 - name: Set up Gradle
   uses: gradle/gradle-build-action@v2
+  with:
+    cache-read-only: ${{ github.ref != 'refs/heads/main' }}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between eea472a and 22ce137.

📒 Files selected for processing (4)
  • .github/workflows/check.yml (1 hunks)
  • .github/workflows/coverage.yml (1 hunks)
  • chat-android/build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/check.yml
  • chat-android/build.gradle.kts
  • gradle/libs.versions.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml

29-29: shellcheck reported issue in this script: SC2086:info:3:32: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (2)
.github/workflows/coverage.yml (2)

1-8: LGTM! Workflow configuration is well structured

The workflow triggers and branch configuration are appropriate for maintaining code coverage standards.


25-27: Verify Kover configuration in Gradle

Ensure that the Kover plugin is properly configured in the Gradle build files.

✅ Verification successful

Kover plugin is properly configured

The Kover plugin is correctly configured in the chat-android/build.gradle.kts file through the plugins block:

plugins {
    alias(libs.plugins.kover)
}

This confirms that the koverXmlReport task used in the GitHub Actions workflow will work as expected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if Kover plugin is configured in Gradle files
echo "Checking Kover configuration in Gradle files..."
rg -l "id\(\"org\.jetbrains\.kotlinx\.kover\"\)" || \
rg -l "kotlin\.code\.coverage"

Length of output: 186


Script:

#!/bin/bash
# Let's check build.gradle files for Kover configuration using different patterns
echo "Checking build.gradle and build.gradle.kts files for Kover..."
fd -e gradle -e gradle.kts | xargs cat

# Also check settings.gradle files for plugin management
echo -e "\nChecking settings.gradle files..."
fd settings.gradle settings.gradle.kts | xargs cat

Length of output: 6159

.github/workflows/coverage.yml Outdated Show resolved Hide resolved
.github/workflows/coverage.yml Show resolved Hide resolved
.github/workflows/coverage.yml Outdated Show resolved Hide resolved
@ttypic ttypic force-pushed the add-coverage-report branch from 22ce137 to f850913 Compare November 28, 2024 11:48
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
.github/workflows/coverage.yml (2)

22-24: Enable Gradle caching for faster builds

While the Gradle setup is correct, enabling caching can significantly improve workflow execution time.

Add caching configuration:

       - name: Set up Gradle
         uses: gradle/gradle-build-action@v2
+        with:
+          cache-read-only: ${{ github.ref != 'refs/heads/main' }}

34-40: Add coverage value logging for better visibility

While the failure condition is properly handled, adding more detailed logging would improve debugging.

Enhance the logging:

       - name: Fail if coverage is too low
         run: |
           MIN_COVERAGE=0.80
+          echo "Current coverage: ${COVERAGE}%"
+          echo "Required minimum: ${MIN_COVERAGE}%"
           if awk "BEGIN {exit !($COVERAGE < $MIN_COVERAGE)}"; then
             echo "Coverage ($COVERAGE) is below the minimum threshold ($MIN_COVERAGE)."
             exit 1
+          else
+            echo "✅ Coverage check passed"
           fi
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 22ce137 and f850913.

📒 Files selected for processing (4)
  • .github/workflows/check.yml (1 hunks)
  • .github/workflows/coverage.yml (1 hunks)
  • chat-android/build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/check.yml
  • chat-android/build.gradle.kts
  • gradle/libs.versions.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml

29-29: shellcheck reported issue in this script: SC2086:info:3:32: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (2)
.github/workflows/coverage.yml (2)

9-11: 🛠️ Refactor suggestion

Define explicit permissions for security

It's recommended to define explicit permissions for the job to follow the principle of least privilege.

Add the following permissions configuration:

 jobs:
   coverage:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      checks: write

28-32: ⚠️ Potential issue

Improve robustness of coverage extraction

The current coverage extraction method has potential issues with XML parsing and variable handling.

Consider using a more robust approach as suggested in the previous review.

🧰 Tools
🪛 actionlint (1.7.4)

29-29: shellcheck reported issue in this script: SC2086:info:3:32: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/coverage.yml Show resolved Hide resolved
@ttypic ttypic force-pushed the add-coverage-report branch from f850913 to 8cb1eda Compare November 28, 2024 12:18
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
.github/workflows/coverage.yml (1)

41-47: Externalize coverage threshold configuration

The minimum coverage threshold is hardcoded in the workflow. Consider making it configurable through repository variables or environment files.

Example implementation:

       - name: Fail if coverage is too low
         run: |
-          MIN_COVERAGE=0.80
+          MIN_COVERAGE="${MIN_COVERAGE:-0.80}"  # Default to 80% if not set
           if awk "BEGIN {exit !($COVERAGE < $MIN_COVERAGE)}"; then
             echo "Coverage ($COVERAGE) is below the minimum threshold ($MIN_COVERAGE)."
             exit 1
           fi
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f850913 and 8cb1eda.

📒 Files selected for processing (4)
  • .github/workflows/check.yml (1 hunks)
  • .github/workflows/coverage.yml (1 hunks)
  • chat-android/build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/check.yml
  • chat-android/build.gradle.kts
  • gradle/libs.versions.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml

29-29: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (3)
.github/workflows/coverage.yml (3)

1-8: LGTM! Workflow triggers are well configured.

The workflow name is descriptive and the triggers are appropriate for coverage monitoring on both PRs and main branch.


9-11: 🛠️ Refactor suggestion

Define explicit permissions for security

It's recommended to define explicit permissions for the job to follow the principle of least privilege.

Add the following permissions configuration:

 jobs:
   coverage:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      checks: write

25-26: ⚠️ Potential issue

Add error handling for Gradle command

The Gradle command execution lacks proper error handling and output verification.

Comment on lines 28 to 43
- name: Extract coverage from XML
run: |
# Extract missed and covered lines from the XML
MISSED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@missed)' chat-android/build/reports/kover/report.xml)
COVERED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@covered)' chat-android/build/reports/kover/report.xml)

# Calculate line coverage percentage
TOTAL=$((MISSED + COVERED))
LINE_COVERAGE=$(echo "scale=2; $COVERED / $TOTAL * 100" | bc)

echo "Line Coverage: ${LINE_COVERAGE}%"
echo "coverage=${LINE_COVERAGE}" >> $GITHUB_ENV

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve robustness of coverage calculation

The current implementation has several potential issues:

  1. Missing error handling for xmllint commands
  2. Unquoted variables in arithmetic operations
  3. No validation of extracted values

Apply this diff to improve robustness:

       - name: Extract coverage from XML
         run: |
+          REPORT_PATH="chat-android/build/reports/kover/report.xml"
+          if [ ! -f "$REPORT_PATH" ]; then
+            echo "Coverage report not found at $REPORT_PATH"
+            exit 1
+          fi
+
           # Extract missed and covered lines from the XML
-          MISSED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@missed)' chat-android/build/reports/kover/report.xml)
-          COVERED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@covered)' chat-android/build/reports/kover/report.xml)
+          MISSED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@missed)' "$REPORT_PATH") || exit 1
+          COVERED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@covered)' "$REPORT_PATH") || exit 1
+
+          # Validate extracted values
+          if ! [[ "$MISSED" =~ ^[0-9]+$ ]] || ! [[ "$COVERED" =~ ^[0-9]+$ ]]; then
+            echo "Failed to extract valid coverage values"
+            exit 1
+          fi

           # Calculate line coverage percentage
-          TOTAL=$((MISSED + COVERED))
-          LINE_COVERAGE=$(echo "scale=2; $COVERED / $TOTAL * 100" | bc)
+          TOTAL=$((MISSED + COVERED))
+          if [ "$TOTAL" -eq 0 ]; then
+            echo "No lines to calculate coverage"
+            exit 1
+          fi
+          LINE_COVERAGE=$(echo "scale=2; $COVERED / $TOTAL * 100" | bc)

           echo "Line Coverage: ${LINE_COVERAGE}%"
-          echo "coverage=${LINE_COVERAGE}" >> $GITHUB_ENV
+          echo "coverage=${LINE_COVERAGE}" >> "$GITHUB_ENV"
📝 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: Extract coverage from XML
run: |
# Extract missed and covered lines from the XML
MISSED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@missed)' chat-android/build/reports/kover/report.xml)
COVERED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@covered)' chat-android/build/reports/kover/report.xml)
# Calculate line coverage percentage
TOTAL=$((MISSED + COVERED))
LINE_COVERAGE=$(echo "scale=2; $COVERED / $TOTAL * 100" | bc)
echo "Line Coverage: ${LINE_COVERAGE}%"
echo "coverage=${LINE_COVERAGE}" >> $GITHUB_ENV
- name: Extract coverage from XML
run: |
REPORT_PATH="chat-android/build/reports/kover/report.xml"
if [ ! -f "$REPORT_PATH" ]; then
echo "Coverage report not found at $REPORT_PATH"
exit 1
fi
# Extract missed and covered lines from the XML
MISSED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@missed)' "$REPORT_PATH") || exit 1
COVERED=$(xmllint --xpath 'string(//report/counter[@type="LINE"]/@covered)' "$REPORT_PATH") || exit 1
# Validate extracted values
if ! [[ "$MISSED" =~ ^[0-9]+$ ]] || ! [[ "$COVERED" =~ ^[0-9]+$ ]]; then
echo "Failed to extract valid coverage values"
exit 1
fi
# Calculate line coverage percentage
TOTAL=$((MISSED + COVERED))
if [ "$TOTAL" -eq 0 ]; then
echo "No lines to calculate coverage"
exit 1
fi
LINE_COVERAGE=$(echo "scale=2; $COVERED / $TOTAL * 100" | bc)
echo "Line Coverage: ${LINE_COVERAGE}%"
echo "coverage=${LINE_COVERAGE}" >> "$GITHUB_ENV"
🧰 Tools
🪛 actionlint (1.7.4)

29-29: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)

@ttypic ttypic force-pushed the add-coverage-report branch from 8cb1eda to f2882f3 Compare November 28, 2024 12:29
Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/coverage.yml (2)

28-30: Improve package installation robustness

Add error handling and quiet flags to apt commands.

-        run: sudo apt-get update && sudo apt-get install -y libxml2-utils
+        run: |
+          if ! sudo apt-get update -q; then
+            echo "Failed to update package list"
+            exit 1
+          fi
+          if ! sudo apt-get install -y -q libxml2-utils; then
+            echo "Failed to install libxml2-utils"
+            exit 1
+          fi

1-50: Consider enhancing coverage reporting capabilities

While the current implementation meets the basic requirement of enforcing 80% coverage, consider these enhancements:

  1. Upload coverage reports as artifacts for historical tracking
  2. Add coverage trend analysis
  3. Generate detailed HTML reports for better visualization
  4. Add per-package coverage metrics

Example step for uploading artifacts:

      - name: Upload coverage report
        uses: actions/upload-artifact@v3
        with:
          name: coverage-report
          path: chat-android/build/reports/kover/
          retention-days: 14
🧰 Tools
🪛 actionlint (1.7.4)

32-32: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb1eda and f2882f3.

📒 Files selected for processing (4)
  • .github/workflows/check.yml (1 hunks)
  • .github/workflows/coverage.yml (1 hunks)
  • chat-android/build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/check.yml
  • chat-android/build.gradle.kts
  • gradle/libs.versions.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml

32-32: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (5)
.github/workflows/coverage.yml (5)

9-11: Define explicit permissions for security

It's recommended to define explicit permissions for the job to follow the principle of least privilege.


12-24: Environment setup looks good!

The setup uses specific versions of actions and appropriate configurations for Java and Gradle.


25-26: Add error handling for Gradle command

The Gradle command execution lacks proper error handling and output verification.


31-43: Improve robustness of coverage calculation

The current implementation has several potential issues with error handling and variable quoting.

🧰 Tools
🪛 actionlint (1.7.4)

32-32: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)


44-50: Coverage threshold check looks good!

The implementation correctly enforces the 80% coverage threshold with clear error messaging.

Copy link
Contributor

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

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

Where can I exactly see the coverage XML file?
and for step
Fail if coverage is low
there is error

awk: cmd. line:1: BEGIN {exit !( < 0.80)}
awk: cmd. line:1:                ^ syntax error
awk: cmd. line:1: BEGIN {exit !( < 0.80)}
awk: cmd. line:1:                       ^ syntax error

@ttypic ttypic force-pushed the add-coverage-report branch from f2882f3 to b384b18 Compare November 28, 2024 12:47
Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (3)
.github/workflows/coverage.yml (3)

11-11: Consider pinning the Ubuntu version

Using ubuntu-latest might lead to reproducibility issues when GitHub updates the runner image. Consider pinning to a specific version like ubuntu-22.04.

-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04

46-46: Make coverage threshold configurable

Consider making the coverage threshold configurable through workflow inputs or environment variables for flexibility.

-          MIN_COVERAGE=80.0
+          MIN_COVERAGE=${MIN_COVERAGE_THRESHOLD:-80.0}

This allows overriding the threshold:

env:
  MIN_COVERAGE_THRESHOLD: 85.0

44-51: Consider adding status checks

The workflow could benefit from creating status checks to display coverage results directly on PRs. Consider using actions like create-status-check or create-pr-comment to improve visibility.

Example addition:

      - name: Create Status Check
        if: github.event_name == 'pull_request'
        uses: actions/github-script@v7
        with:
          script: |
            const coverage = process.env.coverage
            await github.rest.checks.create({
              owner: context.repo.owner,
              repo: context.repo.repo,
              name: 'Coverage',
              head_sha: context.sha,
              status: 'completed',
              conclusion: coverage >= 80 ? 'success' : 'failure',
              output: {
                title: 'Coverage Report',
                summary: `Coverage: ${coverage}%`
              }
            })
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f2882f3 and b384b18.

📒 Files selected for processing (4)
  • .github/workflows/check.yml (1 hunks)
  • .github/workflows/coverage.yml (1 hunks)
  • chat-android/build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .github/workflows/check.yml
  • chat-android/build.gradle.kts
  • gradle/libs.versions.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/coverage.yml

32-32: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)
.github/workflows/coverage.yml (4)

9-11: Define explicit permissions for security

The workflow should define explicit permissions following the principle of least privilege.


25-26: Add error handling for Gradle command

The Gradle command execution lacks proper error handling and output verification.


31-43: Improve robustness of coverage calculation

The current implementation needs improvements in error handling, variable quoting, and value validation.

🧰 Tools
🪛 actionlint (1.7.4)

32-32: shellcheck reported issue in this script: SC2086:info:10:37: Double quote to prevent globbing and word splitting

(shellcheck)


48-51: Improve coverage threshold validation

The current validation using bc might fail in some environments.

Copy link
Contributor

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

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

Can we log code coverage somewhere?
While CI action is passing, it's difficult to find current code coverage: | In terms of percentage as well : (
Maybe echo the XML report as a part of CI logs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
.github/workflows/coverage.yml (2)

3-8: Optimize workflow triggers

Consider refining the triggers to:

  1. Specify explicit PR event types
  2. Add path filters to run only when relevant files change
 on:
   pull_request:
+    types: [opened, synchronize, reopened]
+    paths:
+      - '**/*.kt'
+      - '**/*.kts'
+      - '.github/workflows/coverage.yml'
   push:
     branches:
       - main
+    paths:
+      - '**/*.kt'
+      - '**/*.kts'
+      - '.github/workflows/coverage.yml'

30-30: Pin third-party action version and validate configuration

  1. The mi-kas/kover-report action should use a commit hash for better security
  2. Consider adding coverage trend tracking
-        uses: mi-kas/kover-report@v1
+        uses: mi-kas/kover-report@a5b2c3d4  # Replace with actual commit hash
         with:
           title: Code Coverage
           update-comment: true
           min-coverage-overall: 80
           coverage-counter-type: LINE
+          coverage-badge: true
+          coverage-badge-filename: coverage-badge.svg

Also applies to: 35-37

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b384b18 and 6cc545f.

📒 Files selected for processing (1)
  • .github/workflows/coverage.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/coverage.yml (2)

9-11: 🛠️ Refactor suggestion

Add job-level configurations for security and efficiency

The job configuration needs:

  1. Explicit permissions (for security)
  2. Concurrency control (for efficiency)
 jobs:
   coverage:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      checks: write
+      pull-requests: write
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.ref }}
+      cancel-in-progress: true

The permissions configuration was previously suggested.


25-26: 🛠️ Refactor suggestion

Add error handling for Gradle command

The Gradle command execution needs proper error handling.

Error handling improvement was previously suggested.

Comment on lines +29 to +34
id: kover
uses: mi-kas/kover-report@v1
with:
path: |
${{ github.workspace }}/chat-android/build/reports/kover/report.xml
title: Code Coverage
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve path handling in coverage report step

The current path handling could be more robust:

  1. Use path variables for reusability
  2. Add path existence check
       - name: Add coverage report to PR
         id: kover
         uses: mi-kas/kover-report@v1
+        env:
+          REPORT_PATH: chat-android/build/reports/kover/report.xml
         with:
           path: |
-            ${{ github.workspace }}/chat-android/build/reports/kover/report.xml
+            ${{ github.workspace }}/${{ env.REPORT_PATH }}
+        if: |
+          hashFiles(format('{0}/{1}', github.workspace, env.REPORT_PATH)) != ''

Committable suggestion skipped: line range outside the PR's diff.

Copy link

Code Coverage

Total Project Coverage 80.68%

Copy link
Contributor

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

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

LGTM

@ttypic ttypic merged commit 68f6ca8 into main Nov 28, 2024
3 checks passed
@ttypic ttypic deleted the add-coverage-report branch November 28, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants