ci: run selected example charm integration tests on PRs - #2655
Conversation
e5cee02 to
75ac2d7
Compare
2cada15 to
2e9701e
Compare
2e9701e to
e88dbd5
Compare
tonyandrewmeyer
left a comment
There was a problem hiding this comment.
Thanks for the fork run, it's useful to see it in practice.
I agree a bit of duplication is here is fine, and no worse than the existing duplication we have across the example charms anyway.
I like adding running these, thanks! I think I would have been fine with a less clever system that just ran all the example charms tests if any changed, but this is straightforward to understand and more efficient, so I'm happy to go this way.
james-garner-canonical
left a comment
There was a problem hiding this comment.
I'm not opposed to landing this in unrolled form, but I want to push back -- it leads to a lot of text to scan through, and it makes it really hard to see what's actually different between the tests for the different examples. WDYT about a matrix approach like this?
jobs:
# Detect which example directories have changed on this PR.
detect-changes:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.check.outputs.matrix }}
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
persist-credentials: false
fetch-depth: 0 # Need full history to diff against the base branch.
- id: check
shell: python
run: |
import json, os, subprocess
base = f"origin/{os.environ['GITHUB_BASE_REF']}"
matrix = []
for examples, preset, fetchlibs in [
(["machine-tinyproxy"], "machine", False),
(["k8s-1-minimal", "k8s-2-configurable", "httpbin-demo"], "k8s", False),
(["k8s-3-postgresql", "k8s-4-action", "k8s-5-observe"], "k8s", True),
]:
for d in examples:
cmd = ["git", "diff", "--name-only", f"{base}...HEAD", "--", f"examples/{d}/"]
diff = subprocess.run(cmd, capture_output=True, text=True, check=True)
if diff.stdout.strip():
matrix.append({"example": d, "preset": preset, "fetchlibs": fetchlibs})
output = f"matrix={json.dumps(matrix)}"
print(output) # Logging.
with open(os.environ["GITHUB_OUTPUT"], "a") as f:
print(output, file=f)
test:
needs: detect-changes
# Skip if nothing changed (an empty matrix would fail matrix expansion).
if: ${{ needs.detect-changes.outputs.matrix != '[]' }}
name: ${{ matrix.example }}
strategy:
fail-fast: false # Don't cancel other examples if one fails.
matrix:
include: ${{ fromJSON(needs.detect-changes.outputs.matrix) }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
with:
persist-credentials: false
- name: Set up uv
uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
- name: Set up tox and tox-uv
run: uv tool install tox --with tox-uv
- name: Install Concierge
run: sudo snap install --classic concierge
- name: Prepare for deploying ${{ matrix.preset }} charms
run: sudo concierge prepare -p ${{ matrix.preset }}
timeout-minutes: 90 # Abandon the job if the GHA runner gets stuck.
- name: Fetch charmlibs
if: ${{ matrix.fetchlibs }}
working-directory: examples/${{ matrix.example }}
run: charmcraft fetch-libs
- name: Pack charm
working-directory: examples/${{ matrix.example }}
run: charmcraft pack
- name: Run integration tests
working-directory: examples/${{ matrix.example }}
run: tox -e integrationThat said I haven't tested it and don't really have time to right now. We could capture the idea of matrixifying this in a rainy day issue, I guess?
| concurrency: | ||
| # Cancel in-flight runs when new commits are pushed to the same PR. | ||
| group: example-charm-integration-tests-prs-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
There was a problem hiding this comment.
I'd be fine with dropping this. It can be quite annoying having to wait for integration tests to start from scratch because you pushed a little unrelated docs change or whatever -- and likewise, I think it's bad if you're incentivised to delay pushing to not interrupt the test run.
|
I did some more work on this after @james-garner-canonical's feedback. If we're going to matrix, how about we do away with the detect-changes job. The matrixed job could have a step that decides whether to continue the job. One downside is that there'll be a passing job for each charm, even if a charm's integration tests didn't run. I think that's OK - we mainly care that there are no failing jobs. To cut down the noise on unrelated PRs, we could gate the whole workflow on changes to the The matrixed job could also be made simpler by detecting whether we need to run Here's what the workflow could look like (diff). And a passing run in my fork, which includes trivial changes to k8s-3-postgresql and httpbin-demo. What do you think? |
This PR adds a workflow that runs our example charm integration tests on PRs.
It largely duplicates the existing "Example Charm Integration Tests" workflow that runs on a weekly schedule. The key difference is that the new workflow uses a "detect-changes" job to select which integration test jobs to run, based on exactly which example charms have changed. This avoids needing a separate path-filtered workflow for each charm.
I explored reducing redundancy by extracting the common parts of the scheduled workflow and the new workflow, but the result was hard to understand. I think a bit of duplication is better in this case. Our set of example charms shouldn't change much in future.
Sample run in my fork, where only the
k8s-*charms have changed.