Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
108 changes: 104 additions & 4 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ env:
# Test concurrency: defaults to nproc, capped at 4 (override if needed)
# Note: Flutter 3.32.x may have --concurrency flag behavior issues
FLUTTER_TEST_CONCURRENCY: 4
TOTAL_SHARDS: 4

jobs:
Flutter-Codebase-Check:
Expand Down Expand Up @@ -273,6 +274,15 @@ jobs:
#Get all changed files
ALL_CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRT $BASE_SHA ${{github.event.pull_request.head.sha}} | tr '\n' ' ')
echo "all_changed_files=${ALL_CHANGED_FILES}" >> $GITHUB_OUTPUT

FILE_COUNT=$(echo "$ALL_CHANGED_FILES" | wc -w)
echo "all_changed_files_count=${FILE_COUNT}" >> $GITHUB_OUTPUT

if [ -n "$ALL_CHANGED_FILES" ]; then
echo "any_changed=true" >> $GITHUB_OUTPUT
else
echo "any_changed=false" >> $GITHUB_OUTPUT
fi
- env:
CHANGED_FILES: ${{ steps.changed_files.outputs.all_changed_files }}
if: steps.changed_files.outputs.any_changed == 'true' || steps.changed_files.outputs.any_deleted == 'true'
Expand Down Expand Up @@ -302,10 +312,14 @@ jobs:
exit 1

Flutter-Testing:
name: Testing codebase
name: Run tests (Shard ${{ matrix.shard }})
runs-on: ubuntu-latest
needs:
[Flutter-Codebase-Check, Python-Compliance, check-unused-dependencies]
strategy:
fail-fast: false
matrix:
shard: [0, 1, 2, 3]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand All @@ -318,30 +332,116 @@ jobs:
channel: "stable" # or: 'beta', 'dev' or 'master'
- name: Running pub get to fetch dependencies
run: flutter pub get
- name: Codebase testing
- name: Run tests (shard ${{ matrix.shard }}/${{ env.TOTAL_SHARDS }})
run: |
CONCURRENCY="${FLUTTER_TEST_CONCURRENCY:-$(nproc)}"
if [ "$CONCURRENCY" -gt 4 ]; then CONCURRENCY=4; fi
flutter test --concurrency="$CONCURRENCY" --coverage
flutter test --concurrency="$CONCURRENCY" \
--shard-index=${{ matrix.shard }} \
--total-shards=${{ env.TOTAL_SHARDS }} \
--coverage
- name: Remove generated files from coverage
if: always()
run: |
dart pub global activate remove_from_coverage
dart pub global run remove_from_coverage:remove_from_coverage -f coverage/lcov.info -r '\.g\.dart$'
- name: Upload coverage artifact
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage-shard-${{ matrix.shard }}
path: ./coverage/
retention-days: 1
Comment on lines 343 to +354
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Pin remove_from_coverage to a fixed version for reproducible CI.

Same concern as push.yml: pinning avoids surprise changes from pub.dev.

🤖 Prompt for AI Agents
In @.github/workflows/pull-request.yml around lines 343 - 354, Pin the
remove_from_coverage package to a fixed pub version in the "Remove generated
files from coverage" workflow step: change the activation command for
remove_from_coverage to include an explicit version (e.g.
remove_from_coverage@<version>) so CI uses a reproducible release, and keep the
subsequent dart pub global run remove_from_coverage:remove_from_coverage
invocation unchanged; ensure the chosen version is used consistently for
activation.

Comment on lines +344 to +354
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

if: always() on coverage steps may upload incomplete artifacts.

When a shard's tests fail, the coverage file may be incomplete or missing entirely. Using if: always() uploads potentially corrupt coverage data that could skew merged results.

Consider using if: success() || failure() (excludes cancelled) or checking for file existence before upload.

🛡️ Proposed fix: Conditional upload with file check
       - name: Upload coverage artifact
-        if: always()
+        if: (success() || failure()) && hashFiles('coverage/lcov.info') != ''
         uses: actions/upload-artifact@v4
         with:
           name: coverage-shard-${{ matrix.shard }}
           path: ./coverage/
           retention-days: 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
if: always()
run: |
dart pub global activate remove_from_coverage
dart pub global run remove_from_coverage:remove_from_coverage -f coverage/lcov.info -r '\.g\.dart$'
- name: Upload coverage artifact
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage-shard-${{ matrix.shard }}
path: ./coverage/
retention-days: 1
if: always()
run: |
dart pub global activate remove_from_coverage
dart pub global run remove_from_coverage:remove_from_coverage -f coverage/lcov.info -r '\.g\.dart$'
- name: Upload coverage artifact
if: (success() || failure()) && hashFiles('coverage/lcov.info') != ''
uses: actions/upload-artifact@v4
with:
name: coverage-shard-${{ matrix.shard }}
path: ./coverage/
retention-days: 1
🤖 Prompt for AI Agents
In @.github/workflows/pull-request.yml around lines 344 - 354, The "Upload
coverage artifact" step currently uses if: always() which can upload incomplete
or missing artifacts; update the step that uses actions/upload-artifact@v4
(named coverage-shard-${{ matrix.shard }}) to only run when appropriate by
replacing if: always() with a safer condition such as if: success() || failure()
(to exclude cancelled runs) or add a pre-check that ensures the ./coverage/
directory or coverage/lcov.info exists before uploading; modify the Upload
coverage artifact step's if clause or add a conditional check so the artifact
upload only happens when valid coverage files are present.


Merge-Coverage:
name: Merge Coverage Reports
runs-on: ubuntu-latest
needs: [Flutter-Testing]
if: ${{ !cancelled() }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
distribution: "zulu"
java-version: "17.0"
- uses: subosito/flutter-action@v2
with:
flutter-version: ${{ env.FLUTTER_VERSION }}
channel: "stable"
Comment on lines +377 to +384
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Java and Flutter setup appear unnecessary in Merge-Coverage job.

The Merge-Coverage job only downloads artifacts, merges lcov files, and uploads to Codecov. Java and Flutter are not used in any of these steps.

Removing these setup steps would reduce job execution time by ~30-60 seconds.

♻️ Proposed removal
-      - uses: actions/setup-java@v4
-        with:
-          distribution: "zulu"
-          java-version: "17.0"
-      - uses: subosito/flutter-action@v2
-        with:
-          flutter-version: ${{ env.FLUTTER_VERSION }}
-          channel: "stable"
📝 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-java@v4
with:
distribution: "zulu"
java-version: "17.0"
- uses: subosito/flutter-action@v2
with:
flutter-version: ${{ env.FLUTTER_VERSION }}
channel: "stable"
🤖 Prompt for AI Agents
In @.github/workflows/pull-request.yml around lines 377 - 384, In the
Merge-Coverage job remove the unnecessary Java and Flutter setup steps (the
actions/setup-java@v4 block and the subosito/flutter-action@v2 block) since the
job only downloads artifacts, merges lcov files, and uploads to Codecov; delete
those "uses:" entries and their "with:" subkeys (and verify no other steps in
the Merge-Coverage job reference java-version, distribution, or
env.FLUTTER_VERSION) so the job runs without installing Java or Flutter and
reduces execution time.

- name: Download all coverage artifacts
id: download-artifacts
continue-on-error: true
uses: actions/download-artifact@v4
with:
pattern: coverage-shard-*
path: ./coverage-shards/
merge-multiple: false
- name: Check if artifacts were downloaded
id: check-artifacts
run: |
if find coverage-shards -name "lcov.info" -type f | grep -q .; then
echo "artifacts_found=true" >> $GITHUB_OUTPUT
echo "Coverage artifacts found"
else
echo "artifacts_found=false" >> $GITHUB_OUTPUT
echo "No coverage artifacts found"
exit 1
fi
- name: Install lcov
if: steps.check-artifacts.outputs.artifacts_found == 'true'
run: sudo apt-get update && sudo apt-get install -y lcov

- name: Merge coverage reports
if: steps.check-artifacts.outputs.artifacts_found == 'true'
run: |
mkdir -p ./coverage
echo "Merging coverage from shards..."
find coverage-shards -name "lcov.info" -type f > lcov-files.txt
if [ ! -s lcov-files.txt ]; then
echo "ERROR: No lcov.info files found"
exit 1
fi
echo "Found lcov files:"
cat lcov-files.txt

# Properly merge using lcov to avoid duplicate entries
FIRST_FILE=true
while IFS= read -r file; do
if [ "$FIRST_FILE" = true ]; then
cp "$file" ./coverage/lcov.info
FIRST_FILE=false
else
lcov --add-tracefile ./coverage/lcov.info \
--add-tracefile "$file" \
--output-file ./coverage/lcov.info.tmp
mv ./coverage/lcov.info.tmp ./coverage/lcov.info
fi
done < lcov-files.txt

echo "Merged coverage file size: $(wc -c < ./coverage/lcov.info) bytes"
SF_COUNT=$(grep -c "^SF:" ./coverage/lcov.info || echo "0")
echo "Number of files in merged coverage: $SF_COUNT"
if [ "$SF_COUNT" -lt 50 ]; then
echo "::warning::Only $SF_COUNT files in coverage (expected 50+)"
fi

#######################################################################
# DO NOT DELETE ANY references to env.CODECOV_UNIQUE_NAME in this
# section. They are required for accurate calculations
#######################################################################
- name: Present and upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}}
if: steps.check-artifacts.outputs.artifacts_found == 'true'
uses: codecov/codecov-action@v5
with:
name: "${{env.CODECOV_UNIQUE_NAME}}"
token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: false
verbose: true
exclude: "docs/"
files: ./coverage/lcov.info

- name: Test acceptable level of code coverage
if: steps.check-artifacts.outputs.artifacts_found == 'true'
uses: VeryGoodOpenSource/very_good_coverage@v3
with:
path: "./coverage/lcov.info"
Expand All @@ -350,7 +450,7 @@ jobs:
Android-Build:
name: Testing build for android
runs-on: ubuntu-latest
needs: [Flutter-Testing]
needs: [Merge-Coverage]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand Down
99 changes: 94 additions & 5 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ env:
# Test concurrency: defaults to nproc, capped at 4 (override if needed)
# Note: Flutter 3.32.x may have --concurrency flag behavior issues
FLUTTER_TEST_CONCURRENCY: 4
TOTAL_SHARDS: 4

jobs:
Merge-Conflict-Check:
Expand Down Expand Up @@ -92,10 +93,13 @@ jobs:

Flutter-Testing:
if: ${{ github.actor != 'dependabot[bot]' }}
name: Testing codebase
name: Run tests (Shard ${{ matrix.shard }})
runs-on: ubuntu-latest
needs: Flutter-Codebase-Check
# needs: Update-Documentation
strategy:
fail-fast: false
matrix:
shard: [0, 1, 2, 3]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand All @@ -110,20 +114,104 @@ jobs:
run: cd talawa_lint && flutter pub get && cd ..
- name: Running pub get to fetch dependencies
run: flutter pub get
- name: Codebase testing
- name: Run tests (shard ${{ matrix.shard }}/${{ env.TOTAL_SHARDS }})
run: |
CONCURRENCY="${FLUTTER_TEST_CONCURRENCY:-$(nproc)}"
if [ "$CONCURRENCY" -gt 4 ]; then CONCURRENCY=4; fi
flutter test --concurrency="$CONCURRENCY" --coverage
flutter test --concurrency="$CONCURRENCY" \
--shard-index=${{ matrix.shard }} \
--total-shards=${{ env.TOTAL_SHARDS }} \
--coverage
- name: Remove generated files from coverage
if: always()
run: |
sed -i '/\.g\.dart/d' coverage/lcov.info
- name: Upload coverage artifact
if: always()
uses: actions/upload-artifact@v4
with:
name: coverage-shard-${{ matrix.shard }}
path: ./coverage/
retention-days: 1

Merge-Coverage:
name: Merge Coverage Reports
runs-on: ubuntu-latest
needs: [Flutter-Testing]
if: ${{ !cancelled() }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with:
distribution: "zulu"
java-version: "17.0"
- uses: subosito/flutter-action@v2
with:
flutter-version: ${{ env.FLUTTER_VERSION }}
channel: "stable"
- name: Download all coverage artifacts
id: download-artifacts
continue-on-error: true
uses: actions/download-artifact@v4
with:
pattern: coverage-shard-*
path: ./coverage-shards/
merge-multiple: false
- name: Check if artifacts were downloaded
id: check-artifacts
run: |
if find coverage-shards -name "lcov.info" -type f | grep -q .; then
echo "artifacts_found=true" >> $GITHUB_OUTPUT
echo "Coverage artifacts found"
else
echo "artifacts_found=false" >> $GITHUB_OUTPUT
echo "No coverage artifacts found"
exit 1
fi
- name: Install lcov
if: steps.check-artifacts.outputs.artifacts_found == 'true'
run: sudo apt-get update && sudo apt-get install -y lcov

- name: Merge coverage reports
if: steps.check-artifacts.outputs.artifacts_found == 'true'
run: |
mkdir -p ./coverage
echo "Merging coverage from shards..."
find coverage-shards -name "lcov.info" -type f > lcov-files.txt
if [ ! -s lcov-files.txt ]; then
echo "ERROR: No lcov.info files found"
exit 1
fi
echo "Found lcov files:"
cat lcov-files.txt

# Properly merge using lcov to avoid duplicate entries
FIRST_FILE=true
while IFS= read -r file; do
if [ "$FIRST_FILE" = true ]; then
cp "$file" ./coverage/lcov.info
FIRST_FILE=false
else
lcov --add-tracefile ./coverage/lcov.info \
--add-tracefile "$file" \
--output-file ./coverage/lcov.info.tmp
mv ./coverage/lcov.info.tmp ./coverage/lcov.info
fi
done < lcov-files.txt

echo "Merged coverage file size: $(wc -c < ./coverage/lcov.info) bytes"
SF_COUNT=$(grep -c "^SF:" ./coverage/lcov.info || echo "0")
echo "Number of files in merged coverage: $SF_COUNT"
if [ "$SF_COUNT" -lt 50 ]; then
echo "::warning::Only $SF_COUNT files in coverage (expected 50+)"
fi

#######################################################################
# DO NOT DELETE ANY references to env.CODECOV_UNIQUE_NAME in this
# section. They are required for accurate calculations
#######################################################################
- name: Present and upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}}
if: steps.check-artifacts.outputs.artifacts_found == 'true'
uses: codecov/codecov-action@v5
with:
name: "${{env.CODECOV_UNIQUE_NAME}}"
Expand All @@ -132,6 +220,7 @@ jobs:
verbose: true
exclude: "docs/"
gcov_ignore: "docs/"
files: ./coverage/lcov.info

Android-Build-and-Release:
if: ${{ github.actor != 'dependabot[bot]' }}
Expand All @@ -140,7 +229,7 @@ jobs:
contents: write
environment: TALAWA_ENVIRONMENT
runs-on: ubuntu-latest
needs: Flutter-Testing
needs: Merge-Coverage
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand Down
Loading
Loading