-
-
Notifications
You must be signed in to change notification settings - Fork 576
feat(ci): Implement test sharding (5x testing speed goal part-2) #3281
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
base: develop
Are you sure you want to change the base?
Changes from all commits
2d9b3be
832495e
c69a19d
274fac5
85b2ac4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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: 12 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||
| Flutter-Codebase-Check: | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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' | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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, 4, 5, 6, 7, 8, 9, 10, 11] | ||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||
| - uses: actions/setup-java@v4 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -318,30 +332,141 @@ 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Pin Same concern as push.yml: pinning avoids surprise changes from pub.dev. 🤖 Prompt for AI Agents
Comment on lines
+344
to
+354
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a shard's tests fail, the coverage file may be incomplete or missing entirely. Using Consider using 🛡️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Merge-Coverage: | ||||||||||||||||||||||||||||||||||||||||||||||
| name: Merge Coverage Reports | ||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||
| needs: [Flutter-Testing] | ||||||||||||||||||||||||||||||||||||||||||||||
| if: success() | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check the current state of the pull-request.yml file around line 360
head -370 .github/workflows/pull-request.yml | tail -20Repository: PalisadoesFoundation/talawa Length of output: 608 🏁 Script executed: # Also check what condition is used in push.yml for comparison
grep -n "Merge.*[Cc]overage\|if:.*success\|if:.*cancelled" .github/workflows/push.ymlRepository: PalisadoesFoundation/talawa Length of output: 308 🏁 Script executed: # Get more context around line 360 in pull-request.yml
sed -n '350,370p' .github/workflows/pull-request.ymlRepository: PalisadoesFoundation/talawa Length of output: 649 🏁 Script executed: # Check if there's a Merge-Coverage job definition to understand what should follow
grep -n "Merge-Coverage\|merge.*coverage" .github/workflows/pull-request.yml -iRepository: PalisadoesFoundation/talawa Length of output: 355 Merge-Coverage condition still uses The 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||
| - name: Checkout the Repository | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Fetch base branch for Codecov comparison | ||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||
| set -e | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Fetching base branch: ${{ github.event.pull_request.base.ref }}" | ||||||||||||||||||||||||||||||||||||||||||||||
| if ! git fetch origin ${{ github.event.pull_request.base.ref }} 2>&1; then | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "ERROR: Failed to fetch base branch '${{ github.event.pull_request.base.ref }}' from origin" | ||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Successfully fetched base branch" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| - 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 | ||||||||||||||||||||||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Calculate merge base for Codecov | ||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.check-artifacts.outputs.artifacts_found == 'true' | ||||||||||||||||||||||||||||||||||||||||||||||
| id: get-merge-base | ||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||
| MERGE_BASE=$(git merge-base origin/${{ github.event.pull_request.base.ref }} HEAD) | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Merge base commit: $MERGE_BASE" | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "merge_base=$MERGE_BASE" >> $GITHUB_OUTPUT | ||||||||||||||||||||||||||||||||||||||||||||||
| git show -s --format=%ci $MERGE_BASE | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| ####################################################################### | ||||||||||||||||||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||||||||||||||||||
| fail_ci_if_error: true | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n .github/workflows/pull-request.yml | sed -n '450,475p'Repository: PalisadoesFoundation/talawa Length of output: 1297 🌐 Web query:
💡 Result: On fork-based pull requests, GitHub Actions does not provide repository secrets to the workflow run, so Fix options
Minimal workflow patternsA. Upload only when token exists (skips forks safely): - uses: codecov/codecov-action@v4
if: ${{ secrets.CODECOV_TOKEN != '' }}
with:
token: ${{ secrets.CODECOV_TOKEN }}(Adding the token is the standard approach when you can rely on secrets.) [4][1] B. Tokenless (public repo + token auth disabled): - uses: codecov/codecov-action@v4(Tokenless behavior depends on your Codecov org/repo settings.) [1] If you paste the exact Codecov error lines from the job log, I can tell which of the above applies (token required vs. GitHub API/build lookup vs. config). [2] 🌐 Web query:
💡 Result: In GitHub Actions, pull requests coming from forks do not get access to the upstream repo’s secrets, so What happens next depends on repo visibility / Codecov settings:
If you’re trying to make fork PRs work without exposing
🏁 Script executed: git remote -vRepository: PalisadoesFoundation/talawa Length of output: 321 🏁 Script executed: rg -i "fork\|pull_request_target\|external.contributor" .github/workflows/pull-request.ymlRepository: PalisadoesFoundation/talawa Length of output: 53 🌐 Web query:
💡 Result: The PalisadoesFoundation/talawa GitHub repository is public. [1] Consider handling fork PR coverage uploads differently to prevent blocking external contributions. With
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| verbose: true | ||||||||||||||||||||||||||||||||||||||||||||||
| exclude: "docs/" | ||||||||||||||||||||||||||||||||||||||||||||||
| gcov_ignore: "docs/" | ||||||||||||||||||||||||||||||||||||||||||||||
| files: ./coverage/lcov.info | ||||||||||||||||||||||||||||||||||||||||||||||
| flags: flutter | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - 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" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -350,7 +475,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 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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: 12 | ||
|
|
||
| jobs: | ||
| Merge-Conflict-Check: | ||
|
|
@@ -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, 4, 5, 6, 7, 8, 9, 10, 11] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-java@v4 | ||
|
|
@@ -110,28 +114,133 @@ 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 | ||
| dart pub global activate remove_from_coverage | ||
| dart pub global run remove_from_coverage:remove_from_coverage -f coverage/lcov.info -r '\.g\.dart$' | ||
|
Comment on lines
125
to
+129
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chain🌐 Web query:
💡 Result:
Usage (from the package docs), e.g. to remove generated remove_from_coverage -f coverage/lcov.info -r '\.g\.dart$'(or Citations:
🌐 Web query:
💡 Result:
Practical note: the package docs still show older Sources: 🌐 Web query:
💡 Result: Best practices for version pinning with
|
||
| - name: Upload coverage artifact | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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: success() | ||
| steps: | ||
| - name: Checkout the Repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Fetch base branch for Codecov comparison | ||
| run: | | ||
| set -e | ||
| echo "Fetching base branch: develop" | ||
| if git fetch origin develop:develop 2>/dev/null; then | ||
| echo "Successfully fetched develop branch" | ||
| else | ||
| echo "Warning: Could not fetch develop branch, using current HEAD" | ||
| fi | ||
| - 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 | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| - name: Calculate merge base for Codecov | ||
| run: | | ||
| MERGE_BASE=$(git merge-base origin/develop HEAD || echo "HEAD") | ||
| echo "MERGE_BASE=$MERGE_BASE" >> $GITHUB_ENV | ||
| echo "Using merge base: $MERGE_BASE" | ||
|
Comment on lines
+222
to
+226
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chain🏁 Script executed: rg -n "MERGE_BASE" .github/workflows/push.ymlRepository: PalisadoesFoundation/talawa Length of output: 258 🏁 Script executed: sed -n '220,250p' .github/workflows/push.ymlRepository: PalisadoesFoundation/talawa Length of output: 1280 The The merge base is computed and stored in 🤖 Prompt for AI Agents |
||
|
|
||
| ####################################################################### | ||
| # 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 | ||
| fail_ci_if_error: true | ||
| verbose: true | ||
| exclude: "docs/" | ||
| gcov_ignore: "docs/" | ||
| files: ./coverage/lcov.info | ||
| flags: flutter | ||
|
|
||
| Android-Build-and-Release: | ||
| if: ${{ github.actor != 'dependabot[bot]' }} | ||
|
|
@@ -140,7 +249,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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Matrix shard count should reference
TOTAL_SHARDSto avoid drift.The matrix explicitly lists
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]whileTOTAL_SHARDSis defined separately as12. If someone updatesTOTAL_SHARDSwithout updating the matrix (or vice versa), tests will be incorrectly distributed.Consider using
fromJsonwith a generated sequence or adding a validation step to ensure matrix size equalsTOTAL_SHARDS.♻️ Option: Add validation step to catch drift
Alternatively, document clearly that both must be updated together.
🤖 Prompt for AI Agents