non hidden coverage files fail on merge coverage

[kernelsam]

Tried include-hidden-files: true and it worked as expected. Tried to rename the coverage file and the workflow fails as if it is not finding the files

- name: Rename coverage file
        env:
          COVERAGE_FILE: "coverage.${{ matrix.python-version }}"
        run: |
          mv .coverage "$COVERAGE_FILE"

      - name: Store coverage file
        uses: actions/upload-artifact@v4
        with:
          name: coverage-${{ matrix.python-version }}
          path: coverage.${{ matrix.python-version }}

      - uses: actions/download-artifact@v4
        id: download
        with:
          pattern: coverage-*
          merge-multiple: true

      - name: Coverage comment
        id: coverage_comment
        uses: py-cov-action/python-coverage-comment-action@v3
        with:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          MERGE_COVERAGE_FILES: true

Notice: Generating comment for PR
##[debug]Command failed: args=('coverage', 'combine') path=PosixPath('.') kwargs={} exc.stderr='' exc.returncode=1
Error: Critical error. This error possibly occurred because the permissions of the workflow are set incorrectly. You can see the correct setting of permissions here: https://github.com/py-cov-action/python-coverage-comment-action#basic-usage
Otherwise please look for open issues or open one in https://github.com/py-cov-action/python-coverage-comment-action/
Traceback (most recent call last):
  File "/workdir/coverage_comment/subprocess.py", line 22, in run
    return subprocess.run(
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '('coverage', 'combine')' returned non-zero exit status 1.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/workdir/coverage_comment/main.py", line 44, in main
    exit_code = action(
                ^^^^^^^
  File "/workdir/coverage_comment/main.py", line 96, in action
    return p
##[debug]Docker Action run completed with exit code 1
##[debug]Finishing: Coverage comment

The step before downloads the files without error and downloading them manually works fine as well.

[ewjoachim]

Ah of course, it makes sense coverage combine only knows about coverage files named .coverage<...>.

If you want to do that, you can combine the files yourself with pipx run coverage combine and omit MERGE_COVERAGE_FILES: true. We should document that.

[j-i-l]

coverage combine coverage.* should be the appropriate command in this particular case (though I didn't test it).

When deviating from the .coverage.* syntax the action would probably need to know the pattern used for the individual coverage files, like the COVERAGE_FILE: "coverage.${{ matrix.python-version }}" in this case leading to coverage.* as pattern.

If the idea would be to support this approach, maybe a new option, something like MERGE_COVERAGE_PATTERN might be needed.

As I see it, in the current implementation, the path to where coverage should report on is provided:

https://github.com/py-cov-action/python-coverage-comment-action/blob/8ca9391318803d9f9874d8ba583985de0908e4c7/coverage_comment/coverage.py#L96

This works since coverage combine accepts both paths and files as input (that is expanded before usage, so you can use the wildcard pattern). If a path is provided then it looks for .coverage* in there, if a file is provided it uses this file.

Here below are just some quick and untested drafts, but to implement this feature, Config could get a new attribute (e.g. MERGE_COVERAGE_PATTERN) with None as default and

https://github.com/py-cov-action/python-coverage-comment-action/blob/8ca9391318803d9f9874d8ba583985de0908e4c7/coverage_comment/main.py#L127-L130

could become something like

_, coverage = coverage_module.get_coverage_info(
    merge=config.MERGE_COVERAGE_FILES,
    merge_pattern=config.MERGE_COVERAGE_PATTERN,
    coverage_path=config_path,
)

with https://github.com/py-cov-action/python-coverage-comment-action/blob/8ca9391318803d9f9874d8ba583985de0908e4c7/coverage_comment/coverage.py#L91-L96

becoming

def get_coverage_info(
    merge: bool, merge_pattern: pathlib.Path | None, coverage_path: pathlib.Path
) -> tuple[dict, Coverage]:
    try:
        if merge:
            combine_paths = merge_pattern or coverage_path
            subprocess.run("coverage", "combine", path=combine_paths)

Well, implementing support for non-.coverage* file patterns should probably go to a separate issue, but it might be worth considering, since the include-hidden-files: true approach kind of works against GitHubs effort to keep hidden files out of artifacts.

[ewjoachim]

I'm not sure there is much value for the action dealing with this. I'm not sure there was much value in the action merging the coverage in the first place but it was easy to do.