test: adopt -Werror in unit tests - #203
Open
tonyandrewmeyer wants to merge 7 commits into
Open
tonyandrewmeyer wants to merge 7 commits into
tonyandrewmeyer wants to merge 7 commits into
Conversation
tonyandrewmeyer
requested review from
Deezzir,
H-M-Quang-Ngo,
chanchiwai-ray,
gabrielcocenza,
jneo8 and
xiywang
June 30, 2026 05:43
Add [tool.pytest.ini_options] filterwarnings = ["error", ...] so the unit suite fails on deprecations and resource leaks, with three narrow, message-anchored ignores: - Harness is deprecated (PendingDeprecationWarning): the intentional migration signal; ignored until tests/unit/test_charm.py moves to ops.testing.Scenario. Same precedent as canonical/operator's tox.ini. - Implicitly cleaning up (ResourceWarning): ops Harness TemporaryDirectory teardown leak (operator#2507 class). TEMPORARY: the fix is merged upstream but in no released ops (latest 3.7.1); verified absent against ops main. - GrafanaDashboard._serialize is deprecated (DeprecationWarning): the vendored grafana_agent/v0/cos_agent.py lib calls cosl's deprecated serializer; fix belongs upstream in grafana-agent-operator / cosl.
…bsent _on_collect_unit_status used `get_installed_snap_service(UPSTREAM_SNAP).present` to detect leftover state from charm revision 27. But get_installed_snap_service re-raises SnapNotFoundError when the snap isn't installed, so on every fresh deployment (no legacy snap) the collect_unit_status event handler crashed and the unit landed in error state. Wrap the lookup in a small helper that treats "not installed" as "not present".
Same bug as 341b236 but in _configure: get_installed_snap_service raises SnapNotFoundError when the legacy golang-openstack-exporter snap isn't installed, so fresh deployments errored on cos-agent-relation-changed. Route the lookup through the existing _upstream_snap_present() helper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
remove_upstream_snap and remove_snap_as_resource were unmocked, so a real snap.remove call raised SnapError before the try block, causing lines 94-96 of src/service.py to be skipped (coverage dropped to 99%). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous commit (3ebf49e, "test: mock snap helpers in install-exception test") made a narrow, correct fix to test_snap_install_or_refresh_exception_raises, but alongside it inadvertently reverted most of the rest of the branch: - pyproject.toml: deleted the whole [tool.pytest.ini_options] block, so the branch no longer turned warnings into errors at all -- the entire point of the change -- and reverted the ops pin from ~= 3.8 back to ~= 2.5. - src/charm.py: reverted _upstream_snap_present() back to the inline get_installed_snap_service(UPSTREAM_SNAP).present calls, reintroducing the crash when the legacy snap is absent (SnapNotFoundError escapes) in both _configure and _on_collect_unit_status. - tests/unit/test_charm.py: deleted the regression test covering that. This restores pyproject.toml, src/charm.py, tests/unit/test_charm.py and uv.lock to their state at 8ed8bfb, and keeps the test_service.py mocking fix that commit legitimately added. Unit suite: 156 passed, coverage 100% -- so the coverage gap 3ebf49e set out to close stays closed with the -Werror block back in place.
Contributor
|
The functional test failed because of zaza dependency issue. This remind me may be it's a good timing for us to migrate to jubilant: #206 |
The functional job fails during dependency install, before any test
runs:
x Failed to download and build `prettytable==0.7.2`
|-> Failed to extract archive: prettytable==0.7.2
`-> Archive contains a file with an unsupported compression method;
files must be compressed with 'stored', 'DEFLATE', or 'zstd'
prettytable 0.7.2 (2013) publishes three sdists and no wheel: a .zip, a
.tar.gz and a .tar.bz2. uv.lock pinned the .tar.bz2, which uv cannot
extract, so the install aborts. Point the lock at the .tar.gz instead --
same version, same project, an archive format uv supports.
The version itself is not ours to move: zaza-openstack-tests caps
python-manilaclient below 2.0.0, so 1.29.0 is selected, and that release
requires PrettyTable <0.8.
Not related to the -Werror change in this branch; the same failure
reproduces on main (the scheduled Tests run of 2026-08-15). Verified by
running `uv sync --frozen --group func --no-cache` against a clean
venv: it fails with the exact error above before this change, and
installs all 166 packages after it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Add [tool.pytest.ini_options] filterwarnings = ["error", ...] so the unit suite fails on deprecations and resource leaks, with two narrow, message-anchored ignores:
This bumps ops from 2.x to 3.x -- it seems like that is ok, the tests don't fail and it doesn't conflict with a minimum Python version. If that can't be done, then this change can be done with ops 2.x with one other ignore (or a new version of 2.23 at some point).
Drive-by fix included: the functional job was failing during dependency install, not on anything in this branch, uv.lock pinned prettytable==0.7.2 to its .tar.bz2 sdist, which uv cannot extract. The same failure reproduces on main (scheduled Tests run, 2026-08-15). 6d2cef2 repoints the lock at the .tar.gz sdist of the same version. Happy to split it into its own PR if you'd rather review it separately.