lemurheavy / coveralls-public

The public issue tracker for coveralls.io
http://coveralls.io
124 stars 7 forks source link

Coveralls seems to invalidate other GitHub actions statuses #1739

Open AndrewLister-STFC opened 1 year ago

AndrewLister-STFC commented 1 year ago

I'm trying to update our use of coveralls to use the new Universal Reporter via the official GitHub Action (our coveralls has been failing to report the status for some time and I was hoping this would fix it).

Coveralls now successfully reports coverage but when the status comes through it marks all other checks as "expected" even though they've already run and had the correct status before the coveralls result changed.

Here's a screenshot with the status on the commit expanded to show all of the tests have run and the summary box showing only coveralls results image

and a link to the PR: https://github.com/fitbenchmarking/fitbenchmarking/pull/1206

I'm not sure if this is an issue with how I'm using coveralls, GitHub, or the use of the GitHub API within coveralls. Hopefully I'm just missing something and it's a straight forward fix.

Has anyone seen this before? Is there a known workaround?

afinetooth commented 11 months ago

Hi, @AndrewLister-STFC. Sorry for the delay.

This is very interesting, and some what alarming. I have not seen a case like this before and I am not aware of how one status update could affect others, unless they represent dependencies defined elsewhere, such as in CI.

Can you help me understand if there is any such dependency between your Coveralls upload step and the completion of those other steps?

It feels like received, completed checks would not change state unless they were...sent again?

Do you have any working theories? I have asked around here, but no one is sure just yet.

Perhaps we could run another test?

I could try re-running the build for you to see if that has any effect on the pending statuses?

AndrewLister-STFC commented 11 months ago

No worries @afinetooth, thanks for the help!

The job in the PR that submits the result is below. Nothing uses the needs keyword so no dependencies between jobs. We run a set of tests for linux and then submit the coverage results. I guess that means there's some link between the full_unit_linux check and the coveralls check, but the other jobs are also behaving the same way.

You're welcome to re-run the tests or push some blank commits to trigger them (I can add someone as a dev temporarily) if it would help.

The only thing I can think of trying is some combination of outputs and needs so that the coverage step gets it's own job but that seems a bit more complicated than I had expected.

full_unit_linux:
    name: 'Full Unit Tests (Linux)'
    runs-on: 'ubuntu-latest'
    container:
      image: 'fitbenchmarking/fitbenchmarking-extras:latest'
    steps:
    - name: Checkout code
      uses: actions/checkout@v3
    - name: Installing
      run: |
        sudo -HE --preserve-env=PATH $VIRTUAL_ENV/bin/pip install .[bumps,DFO,gofit,gradient_free,levmar,minuit,SAS,numdifftools,lmfit,nlopt,paramonte]
        sudo -HE --preserve-env=PATH $VIRTUAL_ENV/bin/pip install --upgrade -r requirements.txt
        mkdir -p $MASTSIF
        mkdir -p $PYCUTEST_CACHE
        sudo apt update
        sudo apt install -y curl
    - name: 'Running Tests'
      run: |
        ci/unit_tests.sh
    - name: 'Submit Coverage'
      uses: coverallsapp/github-action@v2
      with:
        file: coverage.lcov
        compare-ref: master
    - name: 'Upload Test Results'
      if: always()
      uses: actions/upload-artifact@v3
      with:
        path: test-results/full_unit_pytest.xml
afinetooth commented 11 months ago

@AndrewLister-STFC I found a clue (or two). I hope it helps:

Clue 1: I noticed that, in your coverage reports, as received by the Coveralls API, the uploads are marked parallel: false, which would be incorrect, since you, apparently, do have a parallel build:

Parallel build:

Screenshot 2023-12-19 at 4 39 02 PM

Coverage Reports uploaded:

7047321887.1:

Screenshot 2023-12-19 at 4 38 14 PM

7047321887.2:

Screenshot 2023-12-19 at 4 36 35 PM

If, indeed, your build is parallel, then you would not only want to indicate that in GitHub Actions Workflow like the parallel usage example here:

    - name: Coveralls Parallel
      uses: coverallsapp/github-action@v2
      with:
        flag-name: whatever
        parallel: true

You would also want to close the parallel build with the final parallel-finished step:

    - name: Coveralls Finished
      uses: coverallsapp/github-action@v2
      with:
        parallel-finished: true
        carryforward: "run-1,run-2"

Clue 2: But, according to your description of your workflow (one job), I suspect your build is not parallel, so the question then becomes: Why did you upload two (2) coverage reports for the same build?

According to your coverage reports, it's because your script did that twice, in two attempts:

7047321887.1:

Screenshot 2023-12-19 at 4 45 00 PM

7047321887.2:

Screenshot 2023-12-19 at 4 45 21 PM

This is partly on us because, while we don't currently, we should probably delete any identical reports when the only difference is a higher service_attempt number.

But, do you have any idea why your workflow would send two version of the same report, if the first one succeeded?

AndrewLister-STFC commented 11 months ago

Thanks for digging into this, that would make sense for why it would invalidate the other results. You're right that I'm not expecting this to run twice, the only reference to coveralls in the script is the job I posted above (coverallsapp/github-action@v2).

I think this might be a red herring though. The dates at the bottom are almost 2 weeks apart, it could be from re-running the tests to see if the behaviour happened again. As soon as the coveralls results are in the other statuses are set to pending so I don't know how this would affect it.

AndrewLister-STFC commented 10 months ago

Watching this happen more closely, the results from coveralls come in exactly when all of the other tests complete (and the results from the other ones disappear). Could it be somehow defining a new set of tests rather than reporting on the running one?

AndrewLister-STFC commented 10 months ago

Sorry for the multiple messages. Looking at the statuses with the github REST API, there are statuses on both the commit which triggered the workflow and the temporary merge commit created to run the tests on.

On the commit there are also status updates from rtd, where the merge commit only has a coveralls status. Is coveralls meant to put statuses on both of these?

Adding the commit ref explicitly seems to have fixed it:

    - name: 'Submit Coverage'
      uses: coverallsapp/github-action@v2
      with:
        file: coverage.lcov
        compare-ref: master
        git-commit: ${{ github.event.pull_request.head.sha }}