test: adopt -Werror in unit tests#771
Open
tonyandrewmeyer wants to merge 3 commits into
Open
Conversation
…from_environ JujuVersion.from_environ() was deprecated in favour of accessing the juju version through the model. The unit tests that mocked it via @patch.object(ops.JujuVersion, "from_environ") are updated to patch ops.model.Model.juju_version (PropertyMock) so they still control whether secrets are reported as supported. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Treat warnings as errors in the unit suite via filterwarnings = ["error", ...] in [tool.pytest.ini_options], with narrow ignores for the warnings the upstream Harness-style suite intentionally still trips: - `Harness is deprecated` — the unit_harness/ suite intentionally stays on Harness, perpetual ignore. - `JujuVersion.from_environ() is deprecated` from the vendored data_platform_libs lib — upstream canonical/data-platform-libs. - `Implicitly cleaning up <TemporaryDirectory>` — Harness tempdir teardown leak (operator#2506/#2507); marked TEMPORARY, drop on next ops 2.23 release backporting the fix, or when this charm bumps to ops >= 3.8. - `unclosed file …yaml` — scenario.Context teardown leak on the charm metadata files (same operator#2506/#2507 class); same TEMPORARY note. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Integration tests surface SSL/websocket unraisable exception warnings from juju/websockets that are out of our control. Override filterwarnings to empty for the integration env so -Werror applies to unit tests only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tonyandrewmeyer
requested review from
minulo and
yanksyoon
and removed request for
a team
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
1 participant
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.
Applicable spec: N/A
Overview
Promote warnings to errors in the unit suite, and clear the one production deprecation it surfaces so the suite is green under it as-pinned.
src/charm.py:_has_secrets()switches from the deprecatedJujuVersion.from_environ()toself.model.juju_version.has_secrets.pyproject.toml: adopt-Werror, with four narrow, message-anchored, individually justified ignores:Harness is deprecated.JujuVersion.from_environ()from the vendoreddata_platform_libslib: module-scoped, upstreamcanonical/data-platform-libs.Implicitly cleaning up <TemporaryDirectory>Harness teardown leak: temporary, drop on the nextops2.23 release that backports the fix or when this charm moves toops >= 3.8.unclosed file …yamlscenario.Contextteardown leak on the charm metadata files, same temporary situation.Rationale
Without
-Werrorthe suite was carrying 97 warnings, including a real production deprecation in_has_secrets(). Flipping turns those into a CI signal so future deprecations and resource leaks fail the suite rather than piling up quietly. Context: discourse post.Juju Events Changes
None.
Module Changes
_has_secrets()reads the juju version fromself.model.juju_versioninstead of calling the deprecatedJujuVersion.from_environ(); same observable behaviour.Library Changes
None.
Checklist
The documentation for charmhub is updateddocs/release-notes/artifacts. If this PR does not require a change artifact, the PR has been tagged withno-release-note. I cannot add tags.urgent,trivial,complex). I cannot add tags.The changelog is updated with user-relevant changes in the format of keep a changelog v1.1.0