Repository navigation
ci: make the broad compatibility tests install the ops wheels - #2773
Draft
tonyandrewmeyer wants to merge 8 commits into
Draft
tonyandrewmeyer wants to merge 8 commits into
tonyandrewmeyer wants to merge 8 commits into
Conversation
Many of the broad charm compatibility jobs passed without ever running the charm's tests against the ops wheels: * The wheels were installed with a bare `pip` in commands_pre, but tox-uv environments have no pip of their own, so the system pip installed them into the user site-packages instead (20 of 89 jobs in the September run). * A `poetry install` in the tox commands put the locked ops back after the wheels were installed. * `uv run` uses the project's own environment, built from uv.lock, rather than the tox environment. Install the wheels with `uv pip install` (which uses the tox environment via VIRTUAL_ENV), with their dependencies, since a charm locked to ops 2 lacks some that ops 3 needs. Install them again after any `poetry install` or `uv sync` in the tox commands. For uv charms, also override ops and its companions in pyproject.toml and relock, so that anything installing from uv.lock gets the wheels, and pass the ops-tracing wheel for that. Add a step that fails the job if any environment the tests created has a different ops version from the wheel, so that another way of bypassing the wheels shows up as a failure rather than a pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An override replaces the extras on every requirement it matches, so taking the extras from the charm's pyproject.toml dropped ones that only a dependency asked for, such as a charm library that requires ops[tracing]. Take them from uv.lock instead, which records every requirement on ops. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tonyandrewmeyer
marked this pull request as draft
September 28, 2026 03:08
Collaborator
Author
|
I'm not sure it's worth doing this, given that I really want to propose the other change, using hyrum. It would make for a better comparison, but I'm not sure that's worth bothering with. |
The check only failed on a mismatched version, so when no environment was created it found nothing and passed. Count the matches and fail on zero, and skip the check when neither the unit nor the static environment ran. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the uv path put the ops-tracing wheel in place, so a poetry or requirements charm on ops[tracing] kept its old ops-tracing next to the new ops, and the check only looked at ops. The tox install now reinstalls ops-tracing from the wheel where the charm already has it, without adding it where it doesn't, and the check compares ops-scenario and ops-tracing with their wheels wherever they're installed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…last Commands that run the unit tests between two locked installs, such as 'poetry install --only unit' before the tests and another install before the static checks, ran the tests against the charm's own ops. The check didn't notice, because by the time it runs the last install has put the wheel back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The script's only feedback was a monthly scheduled run, and it went months without testing the wheels unnoticed. Test the pure parts: the wheel install commands, inserting them after each locked install, the tox allowlist and commands for INI and TOML, and the uv override and its extras. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The workflow already treats the wheel as required, so make the script agree: a missing wheel now fails with argparse's message rather than a bare ls error. Also note in _override_uv_lock that pyproject.toml is rewritten whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the broad charm compatibility tests actually run each charm's tests against the ops wheels. At the moment a lot of the jobs pass without ever doing that: in the September run, 20 of the 89 charm jobs installed the wheels with the system pip (tox-uv environments have no pip of their own, so
pip installwent to the runner's user site-packages), and others put the charm's locked ops back afterwards withpoetry installor ran the tests throughuv run, which uses the project's own environment.uv pip install, which goes into the tox environment viaVIRTUAL_ENV, and with dependencies, since a charm locked to ops 2 doesn't have everything ops 3 needs (opentelemetry, for example).poetry installoruv syncin the tox commands, not just the last one, since some charms run their unit tests between two installs.sh -cwrapper in the tox commands, soshis added toallowlist_externals.pyproject.tomland the charm relocked, so anything installing fromuv.lockgets the wheels. The override keeps the extras from the lock, including ones that only a charm library asks for.test/test_patch_charm_deps.py), since its only feedback so far has been the monthly run, and it went a long time not doing what it said without anyone noticing. This addstomli-wto theunitdependency group, and the tests skip on Python 3.10, which doesn't havetomllib.A run on my fork with an earlier version of this change (https://github.com/tonyandrewmeyer/operator/actions/runs/36358609881) has 11 jobs that passed in September now failing, all of them charms that weren't getting the wheels before:
parca-k8s,zookeeperandzookeeper-k8s,temporal-k8sandtemporal-ui,kafka-k8s, thetempocoordinator, bothmysql-routercharms (whose tests use the Scenario 6 API), andgrafana-agent's static check.k8s-operator's static check now passes. That run predates the ops-tracing and install-after-each changes, so the list will probably shift a bit. I haven't gone through each of those yet to see which are real ops regressions and which are charms that need to update their tests, so the next scheduled run will be noticeably redder, and I'll work through them after this is merged.In that run the new check only caught one charm,
kafka-operator, which runspoetry run pip install -U ./commonafter the wheels are installed and so tests with ops 3.8.2. That one was already failing, and I've left it failing rather than special-casing it.I found this while trialling canonical/hyrum as a replacement for this workflow, which relocks rather than reinstalling and so had been reporting failures here that this workflow wasn't.