Uninett / zino

Zino 2.0 - Network state monitor for research networks
Apache License 2.0
3 stars 5 forks source link

Do not try to publish test results/code coverage in fork #242

Closed johannaengland closed 3 months ago

johannaengland commented 4 months ago

When doing #230 I remove the condition github.repository_owner == 'Uninett' because I thought that was given, but I forgot, that the workflow Run test suite will also be triggered by pushes to the master branch of a fork, which will in turn trigger the Publish test and coverage results workflow. This workflow will then run on the master branch of the fork. And both uploading test results and code coverage will fail since the fork does not have the correct permissions/the code coverage token. Failing workflows can be seen here for publish test results and publish code coverage.

So we do need to restrict this workflow to only run on the Uninett repository.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.98%. Comparing base (b3c2ab8) to head (6358a10).

:exclamation: Current head 6358a10 differs from pull request most recent head 75b30de

Please upload reports for the commit 75b30de to get more accurate results.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #242 +/- ## ========================================== + Coverage 98.97% 98.98% +0.01% ========================================== Files 50 50 Lines 6573 6570 -3 ========================================== - Hits 6505 6503 -2 + Misses 68 67 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hmpf commented 3 months ago

Commit message:

"Prevent publishing test results/code coverage in fork

This only works intermittently so cut down on the noise"

or something similar.

.. or passive-aggressively mention something something about codecov behaving badly =)

johannaengland commented 3 months ago

Commit message:

"Prevent publishing test results/code coverage in fork

This only works intermittently so cut down on the noise"

or something similar.

But publishing test results/code coverage in fork never work, it is not a problem that is fail once in a while, it always fails, because it lacks the necessary permissions

But I can make the commit message a bit more detailed

hmpf commented 3 months ago

"Avoid bothering codecov with code in forks

Codecov does not like forks."

Short, sweet, to the point and suitably #%&/(&%/(.

lunkwill42 commented 3 months ago

"Avoid bothering codecov with code in forks

Codecov does not like forks."

Short, sweet, to the point and suitably #%&/(&%/(.

You guys are discussing and confusing two very different issues here:

  1. Workflows that run in the upstream repo.
  2. Workflows that run in a forked repo.

The issue with no. 1 is when a workflow is triggered by a PR from a forked repo. The workflow runs in the upstream repo, but in the context of the fork (so that things like upstream repo secrets are not available to the workflow). CodeCov supports tokenless uploads for public upstream repos, so the secret isn't needed. However, CodeCov also enforces rate limits for tokenless uploads, and GitHub has many anonymous requests all over for this API, so it becomes a problem for us.

The issue with no 2. is just when you push code to your forked repo, regardless of whether this is associated with a PR in an upstream repo or not: You likely did not configure CodeCov to accept data from the forked repo, so it's never going to work (unless you log in to CodeCov and explicitly add your forked repo as well).

It sounds to me like @johannaengland is discussing no. 2, while @hmpf is annoyed about no. 1.

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud