joshmfrankel / simplecov-check-action

SimpleCov+ Action
MIT License
32 stars 16 forks source link

Coverage check is reported to wrong workflow occassionally #9

Closed clinejj closed 1 year ago

clinejj commented 1 year ago

We have this action set to run in our test workflow, which roughly looks like so:

name: tests

on:
  push:
    branches: [main]
  pull_request:
    branches: [main]

jobs:
  minitest:
    runs-on: ubuntu-latest
    steps:
      # a bunch of setup steps
      - name: Run tests
        id: minitest
        run: |
          bin/rails test
      - name: Upload test artifacts
        uses: actions/upload-artifact@v3
        with:
          name: minitest-artifacts
          path: |
            coverage/**/*.json
          if-no-files-found: 'ignore'
  system_test:
    runs-on: ubuntu-latest
    steps:
      # a bunch of setup steps
      - name: Run tests
        id: system_tests
        run: |
          bin/rails test:system
      - name: Upload test artifacts
        uses: actions/upload-artifact@v3
        with:
          name: system-test-artifacts
          path: |
            coverage/**/*.json
            tmp/screenshots/**
          if-no-files-found: 'ignore'
  coverage:
    runs-on: ubuntu-latest
    needs: [minitest, system_test]
    services:
    steps:
      # a bunch of setup steps and download coverage from other actions
      - name: Merge Coverage Results
        id: coverage
        run: |
          bin/rails coverage:report
      - uses: joshmfrankel/simplecov-check-action@main
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          minimum_suite_coverage: 0
          minimum_file_coverage: 0
          check_job_name: coverage

However, we've noticed that occasionally the check will report to the wrong workflow suite on the PR (it's not consistent which one it reports to). For example, below screenshot reports to our erblint workflow (which is defined in a separate action):

image

This should report under tests / coverage, rather than erblint

joshmfrankel commented 1 year ago

I believe the issue is coming from the fact that both the tests/coverage and erblint/coverage use the same job name. Its possible that either A) there's a bug in this action's code where this the result location is ambiguous (so far all I see is passing the check_name over to Github, so less likely) or B) Github's actions reports the status into the last completed "coverage" job description (a race condition). A quick check would be to change check_job_name: coverage to a different check_job_name OR change erblint's job name to linting/erblint or different naming convention

clinejj commented 1 year ago

I forgot to add here, there is no erblint / coverage job defined (we just have erblint / erblint. Renaming the check_job_name to something unique across all our actions also had the same issue of not consistently reporting to the correct workflow (see below for an updated coverage job that we have):

  coverage:
    runs-on: ubuntu-latest
    needs: [minitest, system_test]
    services:
    steps:
      # a bunch of setup steps and download coverage from other actions
      - name: Merge Coverage Results
        id: coverage
        run: |
          bin/rails coverage:report
      - uses: joshmfrankel/simplecov-check-action@main
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          minimum_suite_coverage: 0
          minimum_file_coverage: 0
          check_job_name: coverage_report
clinejj commented 1 year ago

Doing some digging this is a known problem with the Checks API, where you cannot currently associate a check with a specific workflow:

A possible solution, which is a bit of a different intent of the action, could be to write out the job output to the job summary, which I believe allows for something similar although it's unclear if this shows up on the check summary page:

clinejj commented 1 year ago

Will close this issue since there isn't a fix available, the job summary doesn't post back to the check unfortunately:

image