The test-conda OSX failure is unrelated (#197)

I guess it is quite impossible to test coverage here. I will undo the test changes and try to test this over at astropy instead (astropy/astropy#16419).

But with this patch, you will notice new jobs called Upload Coverage (pull_request) in the checks below. They attempt to download coverage files but skipped uploading because no coverage was done.

Okay... autofix didn't do anything and I failed to see how my diff would trigger any such failures.

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

encode scripts in workflows..............................................Failed
- hook id: encode-scripts
- files were modified by this hook

I see the advantage of this, but it does cause you to loose some fidelity in the codecov reports, as you only get one CI job and you loose all the information about what OS / job it was (I think?).

Can you elaborate on the motivation for this a bit more, is it just codecov being flakey?

cause you to loose some fidelity in the codecov reports

Can you please elaborate on this? Each report would be uniquely named when uploaded as artifacts. They will all be downloaded by artifacts, and then re-uploaded to codecov all together. I don't see it being different from what workflow is already doing except the upload to codecov is a separate step.

motivation

@bsipocz asked for a solution to astropy/astropy#16379

Also it is painful to rerun test suite when only the upload fails.

I don't see it being different from what workflow is already doing except the upload to codecov is a separate step.

I think the difference (and it's not really a deal breaker) is that codecov looses the GHA metadata about which report was from which run?

Anyway, I am happy to merge it, but from the astropy test PR I am unconvinced it actually works?

Alternate solution is welcome. We have been using this over at

https://github.com/spacetelescope/jdaviz/blob/main/.github/workflows/ci_workflows.yml

and

https://github.com/astropy/specutils/blob/main/.github/workflows/ci_workflows.yml

but both workflows only have one coverage job each.

@Cadair , what is this pre-commit silliness?

I think it works as intended now. Please re-review, @Cadair . Thanks!

What was causing the trouble?

Initially, I didn't realize both artifact names actually contains the same coverage.yml filenames, and the artifact name does not really reflect the actual filenames at download time, so they were clobbering each other. But I fixed that.

@Cadair , are you convinced yet? Anything else I need to do here? Thanks!

@Cadair ? @astrofrog ? @ConorMacBride ? 🙏

Thanks!

Hmm, astropy is picking this up but I don't understand why codecov complains about no token. We have one in repo secrets.

https://github.com/astropy/astropy/actions/runs/9355922312/job/25755726624

I think that's a bug with this. The sunpy builds in particular seem to be attempting to upload the same coverage artifacts more than once: https://github.com/sunpy/sunpy/actions/runs/9352703510/job/25742354670#step:3:6

As you can see in that log it's finding jobs from a different invocation of the workflow (the "test" one) and then downloading them in the "docs" workflow. Do you think you will have time to try and fix it @pllim ?

Ah maybe relatedly, that "docs" job on sunpy's CI has pytest: false set, which means it doesn't upload coverage, so if the artifact upload / download was working properly it should fail to download any artifacts at all and then skip the upload to codecov but that clearly isn't working.

Yes this is breaking a lot of CI, a lot of my glue builds as well as reproject are failing. I think the artifact name needs to be more unique as the issue seems to happen for me if multiple OSes test e.g. py311-test