ci: add tests-on-pr workflow file that doesn't upload to codecov

[zmx27]

What problem does this PR address?

In private repositories, the GitHub CI cannot be run properly. For example, there is an issue with uploading coverage to Codecov, which would result in the CI failing. Thus, a new workflow file that follows the centralized workflow in _tests-on-pr.yml but doesn't upload to Codecov is necessary.
Reference: https://github.com/diffpy/diffpy.pdfgetx/pull/134#issuecomment-3066331386
 

What should the reviewer(s) do?

Please review and merge if this looks okay. After this is merged, could you also merge from main into v0?

[zmx27]

@sbillinge this is ready for review.

[sbillinge]

Thanks @zmx27 this looks good. i don't have way to test it really until we try and deploy it.

Before I merge it, can you look and see if there is a more reusable way to do it, for example, create a minimial common workflow that works for the existing case and for this case, and then have separate workflows that capture the different behavior and then make workflows that combine the other workflows. I am not sure how easy that is with GH workflows, but I think @bobleesj did a bit of this work already.

As an example of what I mean, in the current case, there would be a workflow that does everything but doesn't upload to Codecov. Then there would be a workflow that uploads to codecov. Then the current "run tests on pr" would run these two workflows one after the other, and the new one we want here would just not run the codecov part. Do you see what I mean?

[zmx27]

@sbillinge in accordance with our conversation via Slack, I have created a chainable workflow for our tests-on-pr.yml file. Here's how I imagine the process would go. Users can continue to call tests-on-pr.yml in their repos normally; the inputs to the file haven't changed. Inside the tests-on-pr.yml file, it will perform a call to tests-on-pr-no-codecov.yml, which performs all of the steps besides uploading to codecov, and as well as make a call to upload-to-codecov.yml, which would upload coverage to codecov. This makes it so that if we don't want to upload to codecov such as in private repos, we can simply call tests-on-pr-no-codecov.yml. I'm not completely sure I have the yaml syntax right, so please review!

[sbillinge]

@sbillinge in accordance with our conversation via Slack, I have created a chainable workflow for our tests-on-pr.yml file. Here's how I imagine the process would go. Users can continue to call tests-on-pr.yml in their repos normally; the inputs to the file haven't changed. Inside the tests-on-pr.yml file, it will perform a call to tests-on-pr-no-codecov.yml, which performs all of the steps besides uploading to codecov, and as well as make a call to upload-to-codecov.yml, which would upload coverage to codecov. This makes it so that if we don't want to upload to codecov such as in private repos, we can simply call tests-on-pr-no-codecov.yml. I'm not completely sure I have the yaml syntax right, so please review!

yup, that's exactly what I had in mind. I am not expert on the yaml so we will need to test it. I can merge and we can test by trying to get it to run on a new commit. Please test that the old test-on-pr runs as expected. We can also see if @bobleesj can look it over.

My one comment is that I think it is probably a bad idea to use a relative path for the uses. I see that in other files we do it both ways and @bobleesj can comment if this is important or not, but in general i think it may be more robust as scikit-package/release-scripts/.github/workflows/.... Please could you change it to that? Then I can merge it and we can test it.

[sbillinge]

please see comment

[sbillinge]

these need an @v0 at the end of the line.

please crosscheck carefully with other workflows using this construction in this repo for any other details. I am not sure how the pass-through of inputs.<whatever> work as it is not clear to me how these workflows pass this info through

[zmx27]

From what I understand from the GitHub Docs, the with keyword will pass in the inputs to the called reusable workflow. Here's how I thought about it: in our case, we'd be calling _tests-on-pr.yml from another repo with certain inputs project, headless, etc, and these are going to be stored as inputs.<whatever> inside _tests-on-pr.yml. These values (and secret tokens in the next codecov section) then gets passed as the inputs to the next reusable workflow (_tests-on-pr-no-codecov.yml and _upload-to-codecov.yml) where it is then used to run all the jobs stated there. Feel free to correct me if I'm wrong about any of this!

[sbillinge]

@zmx27 one more thing. We will need a FAQ for this. Something like I have a private repo and don't have a Codecov paid plan. Can I modify the CI workflows for this situation? and then explain the steps (...manually edit the such and such workflow, changing such and such to so and so.

That would have to go into scikit-package, so maybe now just make an issue and we can work on it when we have completed this work and tested it.

[zmx27]

@sbillinge this is ready for review. I will also make an issue in scikit-package.

@bobleesj if you have time, could you please take a look at this as well? Thank you so much!

[bobleesj]

Thanks for the initiative. Just to confirm, what would be different from the other test on pr workflow without codecov upload used for Level 4 (system)?

[zmx27]

This is a more generalized centralized workflow that will also perform the necessary jobs in the case that c_extension and headless is inputted as true. I believe that the other tests-on-pr workflow used for skpkg-system does not have either of these options and therefore is much more niche.

[bobleesj]

(I didn't look at this carefully yet since I am on the road but will check after your response)

[sbillinge]

Thanks for the discussion. This looks good for testing. A question i have is how to do that. Obviously, I can merge this and merge v0 and we can just do some test commits in different repo's that do use the different aspects to test the templating pass-throughs. (dfifpy.fourigui uses headless and there are a few that use c extenzions. However, is there a cleaner way to do it?

I would be happy to do it this way if we think it is going to work out of the box with very minor tweaks, but if there is a "debugging" process this will be very clunky. @bobleesj how have you been doing this process?

[sbillinge]

also, @bobleesj can you confirm that it works to pass the templates through to these workflows called by workflows further up the chain? I know that at the repo level it is known what input.<something> is and so can correctly build {{ input.<something> }} into <the-value-of-input.thing>, but it is not clear to me when that happens and whether we can pass through to these these variables to the lower level workflows and they are still available. If that is the case, this would be really super (and a bit magical!)

[zmx27]

@sbillinge I've reverted this PR to our previous version without the chained workflows as per our discussion in our Zoom meeting. This is ready for review.

[sbillinge]

thanks. please could we just do that on a clean PR?

[sbillinge]

sorry to create extra work!