fgrosse / go-coverage-report

A CLI tool and GitHub Action to post Go code coverage reports as comment to your pull requests.
BSD 3-Clause "New" or "Revised" License
59 stars 11 forks source link

No coverage report posted when only _test.go files changed. #18

Closed ChrisHines closed 6 months ago

ChrisHines commented 6 months ago

We're using this action on several private repositories and it is working great so far. Thanks.

However, I made a PR today that only changed a single _test.go file and no coverage report was added to the PR. The changed test file was noticed by the first steps of the action but seems to get filtered away sometime later. Here are the logs (somewhat redacted).

Run tj-actions/changed-files@aa08304bd477b800d468db44fe10f6c61f7f7b11 <snip>
changed-files
  Warning: The current working directory is not inside a git repository: <snip>
  Using GitHub's REST API to get changed files
  Getting changed files from GitHub API...
  Found 1 changed files from GitHub API
  All Done!
changed-files-patterns
  All Done!
Run $GITHUB_ACTION_PATH/scripts/github-action.sh <snip>
Download code coverage results from current run
Download code coverage results from target branch
Compare code coverage results
  + go-coverage-report -root=<snip> -trim= .github/outputs/old-coverage.txt .github/outputs/new-coverage.txt .github/outputs/all_changed_files.json
  Skipping report since there are no changed files
  + end_group
  Notice: No coverage report to comment
fgrosse commented 6 months ago

Hi Chris, test files are filtered out during the Determine changed files step here:

https://github.com/fgrosse/go-coverage-report/blob/a284a4c69b3383da62629009e9e8e58976efbf6a/action.yml#L54-L64

I did this to provide a clean table of coverage changes per file (i.e. the "Coverage by file" table in the coverage comment which shows number of covered statements per changed file etc). However, I didn't think about coverage changes that only result from changes to the unit tests.

As a fix, I will remove the **_test.go from the ignored files. I'm not certain though, what I want to do with the "Coverage by file" table. Mixing unit tests and other Go files in the same table seems a bit confusing to me so maybe two separate tables would be best? The table for unit tests wouldn't really contain more information than the name of the changed file though :thinking: Any thoughts on this?

This is how such a table would currently look like in the case of only a single unit test file being changed:

Changed File Coverage Δ Total Covered Missed :robot:
github.com/fgrosse/prioqueue/min_heap_test.go 0.00% (ø) 0 0 0
fgrosse commented 6 months ago

I decided to go ahead with this format: https://github.com/fgrosse/go-coverage-report/pull/20#issuecomment-2079113007 which simply displays test files as a separate list below the existing table.

fgrosse commented 6 months ago

The issue is fixed via v1.0.1 :tada:

ChrisHines commented 6 months ago

Thanks for the quick fix! 🥇

I agree with splitting the test files into a separate section. My team was a little confused why they were included in the Coverage by file section since test files don't get test coverage themselves.