-
Notifications
You must be signed in to change notification settings - Fork 2.1k
GitHub Actions tests workflow improvements #8783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Modify the test workflow to align it with the test-infrastructure scripts, namely: * Run on the `python:3.10-bookworm` image, the same used in the local test docker compose and the official Docker images * Use the exiting `install_deps.sh` and `init_environment.sh` scripts to have a consistent setup * Use consistent service names and adapt the ini file (test-core-github-action.ini) so it can be used in both environments Other than that, cleanup the workflow file, specially around env vars set
Use apt-get to avoid warning, install latest setuptools as per #8699
@duttonw the only thing I haven't managed to fix is the failure in the Test Result Combine job. If you have any ideas on how to fix it that would be great but to be honest I'm tempted to remove that whole section or just keep the console report. Having codecov available I see little benefit to generate all these reports in HTML and other artifacts when 99.99% of the time we won't look at them. I see your points in this comment but these commands add quite a lot of complexity to the workflow for someone who is not familiar with it, and if Codecov starts becoming an issue we can always revisit and re-implement them. |
I've switched off the Circle CI builds so hopefully that will stop the checks being run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've provided comments, some are optional (comment update etc)
The reason you are having major issues is that the coverage system is designed
to be run on the same 'ecosystem' that generated the sqlite db file from pytest.
Since you are mixing container runtime and root GitHub Actions, they have different fully qualified paths and as such it unable to find the source code that generated the coverage artifact.
tl;dr: Use the same container in merging as what made the coverage database.
Possible Please don't do this via trial and error change the working directory to the same path as whats in the container used for git checkout (this sadly may be a fools errand since you are not even remotely in the same path structure and it could be rejected due to clashes with other container runners)/
@@ -1,6 +1,6 @@ | |||
# Local testing | |||
|
|||
This is a set of scripts that replicate the contents of `/.circleci/config.yml`, enabling testing on the local machine with the same setup as circleci. It requires docker with the compose plugin to be installed and on the path, as well as the `sh` shell. It should work out of the box on linux/mac, Windows is unknown. | |||
This is a set of scripts that replicate the contents of `/.github/workflows/pytest.yml`, enabling testing on the local machine with the same setup as GitHub Actions. It requires docker with the compose plugin to be installed and on the path, as well as the `sh` shell. It should work out of the box on linux/mac, Windows is unknown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add comment about it being best to fork to your personal enviornment as it provides a test running that won't compete with other users.
I have noted on my qld-gov-au org as well as seeing it on the ckan org that if your doing testing, use your own person account when its public public, then bring back into the org for final testing.
test-core-github-actions.ini
Outdated
@@ -4,14 +4,14 @@ use = config:test-core.ini | |||
[app:main] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a blurb at the top of this that this requires to be run in a containerised environment OR set /etc/host paths for the following....
@duttonw Thanks for your suggestions and the changes proposed in #8787. I've picked a few of them but left some out. Most notably the building of Solr images adds a lot of complexity for a really rare scenario as we don't touch the Solr schema that much (and we will hopefully stop doing it with the new search). I think we can leave as it is now, merge this and backport it to 2.11. and 2.10 and do gradual improvements over time, leaving folks time to familiarize themselves with the workflow. @kowh-ai we need to get this into 2.11/2.10 soon as they are not currently tested and we need tests in place to prepare the next patch release so let me know if you have major concerns about anything. |
AFAICT this looks solid...will merge |
This is a backport of ckan#8783
Modify the test workflow to align it with the test-infrastructure scripts, namely:
python:3.10-bookworm
image instead of the github runner with a custom setup, the same used in the local test docker compose and the official Docker imagesinstall_deps.sh
andinit_environment.sh
scripts to have a consistent setuptest-core-github-actions.ini
) so it can be used in both environmentsOther than that, cleanup the workflow file, specially around env vars set, and update the docs to reflect the Circle CI -> GitHub Actions migration