Improve ci.yml, enhance testability, and document the testing process
#41
Conversation
Changes: - Remove the needless `preflight_check_dists` job - Merge the wheel testing step into upload_to_testpypi - Add the use of `check-wheel-contents` to the tests - Make various changes to better support local testing - Simplify some of the workflow syntax used
Update and add more details about the testing process.
If an error is reported, it's easier if it's obvious which of the two programs produced it.
This will make the command show up in the run log as its own entry, making it easier to locate in the output.
Some of the settings for building wheels can be put into `pyproject.toml`. This fits with current best practices and lets us further simplify `ci.yml`.
Turns out `vars.local_testing` is never needed.
`check-wheel-contents` complains about the wheel only on Windows and not other architectures. I suspect there is a bug in `check-wheel-contents`. Better to skip it for now.
mhucka
changed the title
ci.yml, enhance testability, and document the testing process
mhucka
added
the
devops
I can't think of a good reason to cause the job to fail if an existing file from the same release is already on pypi. This might happen if you try to rerun the workflow because of a failure during one of the files. Let's just let it give a warning and move on.
I realized that the values should be left unset in the default case.
|
|
||
| ```dockerfile | ||
| # Start from a base image that is already configured for act. | ||
| FROM catthehacker/ubuntu:act-latest |
There was a problem hiding this comment.
catthehacker?
Should this be pinned?
There was a problem hiding this comment.
Yeah, I know, catthehacker sounds dodgy :-). It's the moniker of the user who builds Docker images for use with act: https://nektosact.com/usage/runners.html
Good point about pinning that with a hash. I will also see if I can find an alternative, because if nothing else, this invites suspicion.
|
|
||
| ```shell | ||
| rm -rf /tmp/act-artifacts/* | ||
| ``` |
There was a problem hiding this comment.
This seems like quite a lot of instructions for "run ci locally". They also look like instructions that will become out of date in a year or two due to some detail changing (like mentioning specific github usernames and python packages and cache locations).
Basically I have low confidence that if I read this in a year that it will work, which is perhaps unavoidable, but maybe an alternate strategy like linking to a "how to run github ci" documentation page within github is possible? Ironically my instinct is to take a complex step like this and ensure the process specified in the documentation is tested in ci so if something becomes wrong it flags immediately.
There was a problem hiding this comment.
linking to a "how to run github ci" documentation page within github
I wish there was! But so far I haven't found anything that makes all these details explicit. People can work it out, but because it took some effort for me to figure it out, it made sense to write it down (otherwise I'll forget how it was done). And once I started writing notes, it wasn't much more work to include a readme here.
It's true it's a lot of instructions. Not all are really necessary for Chromobius; for example, contributors don't really need to test uploading wheels to a PyPI index for a normal local run. Overall, maybe it would make more sense to put this information elsewhere. What about:
- the wiki on GitHub?
- a Google doc?
W.r.t. having tests for this, that's a great idea, but maybe that can be left for future work?
| {os: ubuntu-latest, dist: cp312-manylinux_x86_64, arch: x86_64}, | ||
| {os: ubuntu-latest, dist: cp313-manylinux_x86_64, arch: x86_64}, | ||
| {os: ubuntu-latest, dist: cp314-manylinux_x86_64, arch: x86_64}, | ||
| {os: ubuntu-24.04, dist: cp310-manylinux_x86_64, arch: x86_64}, |
There was a problem hiding this comment.
Wasn't this changed to latest last commit?
There was a problem hiding this comment.
These workflows are a bit frustrating. On one hand, it's good to test against the latest runners in order to catch breakage when something changes; OTOH, it's better for reproducibility to use a specific version. Most sources (including Google's) recommend using specific versions, so I thought it better to go back to doing that. (Do you prefer using -latest instead?)
In principle, that variable shouldn't need to be set. Let's try removing it.
2 participants
Changes:
Move more
cibuildwheelsettings into thepyproject.tomlfile, which both aligns with recommended practices and allows simplifications toci.yml.Get rid of
preflight_check_distsjob inci.ymland make thetwine checkstep be part of theupload_to_testpypijob. This simplifies and streamlines the workflow.Get rid of references to
vars.ACTinci.yml, which turn out to be unnecessary.Enhance local testability by adding support for using a local pypi server for testing in the
upload_to_testpypijob.Add an input variable
upload_to_pypitoworkflow_dispatchinvocations to allow control over whether the upload topypi.orgis done during manual invocations.Set
skip-existingtotruewhen usingactions/gh-action-pypi-publishso that partial reruns of the workflow are possible.Use explicit versions of runners instead of
-latest.Make miscellaneous small tweaks to
ci.yml.Add a README file to
.github/workflows/detailing how to do local testing ofci.yml, to document the process.