From 2d0cb5798129643ef4df7126ffe5544baff33f04 Mon Sep 17 00:00:00 2001 From: PhiLily <252857470+PhiLily@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:54:12 +0200 Subject: [PATCH] Write one test's tzdata, keep pytest below 9.2, patch _commit once One test in tests/test_import_beat.py asserted that ox_import_beat_schedules treats US/Eastern and America/New_York as one zone by reading both from the installed tz database and comparing their bytes. The command counts two keys as one zone only when their data files are identical, since equal offsets today say nothing about either zone's future rules, so the test passed or failed with the installed tzdata rather than with the code. The test now writes the same synthetic TZif data under both keys through the zone_files fixture, the way test_two_names_for_the_same_zone_data_are_one_zone in the same file already does, so the bytes it compares are the test's own. The real zone names stay, since the point is that this legacy-name pair is translated as one zone. The dev dependency group now requires pytest>=8.0,<9.2. pytest 9.2.0 changed how monkeypatch.setattr records the value it will restore: it reads that value from the target's own __dict__ rather than through getattr, so that patching an attribute an instance inherits from its class no longer leaves a shadowing entry on the instance at teardown. Django's proxies, django.contrib.admin.sites.site and django.db.connection among them, delegate every attribute to a wrapped object and hold none of those attributes in their own __dict__, so for them that lookup finds nothing every time, and teardown deletes the attribute from the wrapped object instead of restoring it. Where the wrapped class defines the attribute, as DatabaseWrapper defines _commit, the deletion uncovers the class's own and nothing is lost. Where only the wrapped instance has it, as AdminSite has _actions, the deletion is permanent, and every later test in the process that reads it fails. tests/test_schedule_admin_unreadable_actions.py patches site._actions. The pin excludes the whole 9.2 line rather than 9.2.0 alone, so a later 9.2 release with the same behaviour cannot reach the suite. It stands until a release restores attributes patched through a proxy. The comment beside the pin in pyproject.toml records this. Two tests in tests/test_dispatch_errors.py, test_a_commit_that_fails_is_not_a_callback_failure and test_a_commit_that_loses_the_connection_ends_the_pass, replaced django.db.connection._commit through monkeypatch.setattr, and the replacement itself called monkeypatch.setattr on the same attribute to put the original back. That puts two undo entries for one attribute on monkeypatch's stack, and nothing about its teardown promises to unwind them into the state either test wants: with the bookkeeping above, both entries record the attribute as absent, teardown deletes it twice, and the second deletion raises AttributeError. Each test now patches _commit once, and the replacement calls the captured original directly: the first raises on its first call and commits on every later one, the second ends the session before calling the original. No application code changes. --- pyproject.toml | 14 +++++++++++++- tests/test_dispatch_errors.py | 20 +++++++++++++++++--- tests/test_import_beat.py | 14 +++++++++++--- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index fa5cd92..3872f0a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,19 @@ Issues = "https://github.com/oxpull/django-ox/issues" [dependency-groups] dev = [ - "pytest>=8.0", + # Ceiling: 9.2.0 changed monkeypatch.setattr's undo bookkeeping to read + # the pre-patch value from the target's own __dict__ (pytest #10644). + # Through a Django LazyObject proxy (django.contrib.admin.sites.site is + # one; django.db.connection is another), the delegated attribute is + # never in that __dict__, so monkeypatch records it as absent and + # deletes it from the wrapped object at teardown instead of restoring + # it. Harmless where a class method shows through once the instance + # shadow is gone (DatabaseWrapper._commit), but an attribute that exists + # only on the wrapped instance, such as AdminSite._actions, is then gone + # for every later test in the process. Upstream tracks it as + # pytest-dev/pytest#15099, open with no fix yet, so the ceiling is the + # whole 9.2 line rather than naming just 9.2.0. + "pytest>=8.0,<9.2", "pytest-django>=4.9", "pytest-cov>=6.0", "ruff>=0.14", diff --git a/tests/test_dispatch_errors.py b/tests/test_dispatch_errors.py index d41c218..b66415b 100644 --- a/tests/test_dispatch_errors.py +++ b/tests/test_dispatch_errors.py @@ -413,10 +413,22 @@ def test_a_commit_that_fails_is_not_a_callback_failure( # the failure is that schedule's, and the pass goes on to the next. with_history() commit = connection._commit + fired = False def failing_commit(): - monkeypatch.setattr(connection, "_commit", commit) - raise OperationalError("server closed the connection unexpectedly") + # Fires once, then calls the real commit directly for every + # later call in this test. Re-patching the attribute from here + # (rather than calling `commit` straight) left monkeypatch's own + # undo stack holding two entries for the same (connection, + # "_commit") pair; pytest 9.2's teardown unwinds that as a + # delattr on an attribute the second entry had already removed, + # raising AttributeError instead of restoring it. Never re-enter + # monkeypatch.setattr on the attribute it is already tracking. + nonlocal fired + if not fired: + fired = True + raise OperationalError("server closed the connection unexpectedly") + return commit() monkeypatch.setattr(connection, "_commit", failing_commit) with caplog.at_level(logging.INFO, logger="django_ox"): @@ -438,7 +450,9 @@ def test_a_commit_that_loses_the_connection_ends_the_pass( commit = connection._commit def commit_on_a_dead_session(): - monkeypatch.setattr(connection, "_commit", commit) + # Calls the real commit directly; see the note in + # test_a_commit_that_fails_is_not_a_callback_failure above about + # never re-entering monkeypatch.setattr on this same attribute. kill(connection) return commit() diff --git a/tests/test_import_beat.py b/tests/test_import_beat.py index f5de7cd..43eac14 100644 --- a/tests/test_import_beat.py +++ b/tests/test_import_beat.py @@ -963,10 +963,18 @@ def test_a_schedule_in_another_timezone_is_not_translated(beat_tables): assert "nightly" not in printed_rows(output) -def test_an_equivalent_zone_under_another_name_is_still_translated(beat_tables): - # US/Eastern and America/New_York are one zone. Comparing the strings +def test_an_equivalent_zone_under_another_name_is_still_translated( + beat_tables, zone_files +): + # US/Eastern and America/New_York are one zone in the tz database: a + # link to the same rules under a legacy name. Comparing the strings # would divert a correctly-aligned schedule into the untranslated list. - # The one test here that leans on the tz database's own links. + # Built here under both names, so which releases of the system or + # packaged tzdata the link survives in is not this test's business; + # see test_two_names_for_the_same_zone_data_are_one_zone for the same + # mechanism under names the tz database has no opinion on at all. + zone_files("US/Eastern", PLUS_TWO) + zone_files("America/New_York", PLUS_TWO) set_crontab(1, timezone="US/Eastern") with override_settings(TIME_ZONE="America/New_York"): output = run()