fix: close SQLite storage in Harness.cleanup()#2507
Conversation
Harness held a TemporaryDirectory and an SQLiteStorage that were only released when the object was garbage-collected. On Python 3.14 those fallbacks emit ResourceWarning (Implicitly cleaning up TemporaryDirectory, unclosed database), which pytest wraps as PytestUnraisableExceptionWarning and -W error turns into a test failure. This was a major contributor to downstream charm test failures under -Werror in canonical/hyrum runs. Mirror the approach being taken for scenario.Context in canonical#2506: replace the two TemporaryDirectory instances with mkdtemp + weakref.finalize so the GC-time cleanup path is silent, and register a finalizer that closes Harness's Framework (and thus the SQLite connection) if the caller forgot to call cleanup(). cleanup() itself remains the eager-cleanup entry point and is idempotent. Adds two regression tests: one asserting cleanup() releases the tempdir, and one asserting that dropping a Harness without cleanup() does not emit any ResourceWarning.
tonyandrewmeyer
changed the title
To help my understanding: Are those charm tests not using |
There are two pieces in the PR:
Thinking about this more, I think the second case should be dropped; people should explicitly call |
…people to properly call cleanup.
tonyandrewmeyer
changed the title
|
Hmm, that data failure looks like it may be a real issue. Moving to draft while I figure that out. |
It's a downstream bug. We should wait for the PR with a fix to land before merging this, to avoid disabling our CI. There's no particular urgency. |
2 participants
We don't normally do fixes in Harness, but this will allow charms that are not yet migrated to Scenario to run with
-Werrorand not get hit by a resource warning we cause.Harnessbuilds aframework.Frameworkbacked by an in-memorySQLiteStorage, butHarness.cleanup()never closes it. The sqlite3 connection is only released when theHarnessis garbage-collected, at which point its destructor emitsunclosed database in <sqlite3.Connection ...>. pytest wraps that asPytestUnraisableExceptionWarningand-W errorturns it into a test failure — even for callers who correctly callharness.cleanup().This is one of the bug classes flagged by a recent
canonical/hyrum(super-tox) run across the canonical charm collection: under-Werror, charms whose tests useops.testing.Harnessregress even though their own code is clean.Changes:
Harness.cleanup(): callself._framework.close()after the backend cleanup so theSQLiteStorageconnection is closed eagerly. sqlite3'sclose()is idempotent, so repeatedcleanup()calls stay safe.harness.cleanup().