Skip to content

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

Merged
merged 12 commits into from
Apr 15, 2025
Merged

Conversation

amercader
Copy link
Member

@amercader amercader commented Apr 10, 2025

Modify the test workflow to align it with the test-infrastructure scripts, namely:

  • Run on the 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 images
  • Use the existing 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-actions.ini) so it can be used in both environments

Other than that, cleanup the workflow file, specially around env vars set, and update the docs to reflect the Circle CI -> GitHub Actions migration

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
@amercader
Copy link
Member Author

@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.

@amercader
Copy link
Member Author

I've switched off the Circle CI builds so hopefully that will stop the checks being run

Copy link
Contributor

@duttonw duttonw left a 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.
Copy link
Contributor

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.

@@ -4,14 +4,14 @@ use = config:test-core.ini
[app:main]
Copy link
Contributor

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....

@amercader amercader changed the title GitHub Actions tests workflow improvments GitHub Actions tests workflow improvements Apr 11, 2025
@amercader
Copy link
Member Author

@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.

@kowh-ai
Copy link
Contributor

kowh-ai commented Apr 15, 2025

AFAICT this looks solid...will merge

@kowh-ai kowh-ai merged commit 8ac2c11 into master Apr 15, 2025
44 of 45 checks passed
@kowh-ai kowh-ai deleted the new-tests-ci-changes branch April 15, 2025 14:47
amercader added a commit that referenced this pull request Apr 23, 2025
This is a backport of #8783
amercader added a commit that referenced this pull request Apr 23, 2025
This is a backport of #8783
duttonw pushed a commit to qld-gov-au/ckan that referenced this pull request May 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants