Skip to content

Commit 370b0b8

Browse files
test: strengthen focused unit coverage for core behavior
Cover PR processing, provider helpers, review output rendering, configuration behavior, retry handling, ticket extraction, code suggestions, and GitHub app workflows. These tests target behavior that is important and likely to change, while keeping production code untouched. They also isolate shared test state so the new coverage remains order-independent. This work was developed through multiple rounds of human-led AI collaboration using codex-cli v0.130.0 with GPT-5.5, GitHub Copilot CLI v1.0.48 with GPT-5.5, and Claude Opus 4.7.
1 parent 0bd56c0 commit 370b0b8

17 files changed

Lines changed: 5506 additions & 1 deletion

pr_agent/git_providers/utils.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ def apply_repo_settings(pr_url):
3535
category = 'local'
3636
try:
3737
fd, repo_settings_file = tempfile.mkstemp(suffix='.toml')
38-
os.write(fd, repo_settings)
38+
try:
39+
os.write(fd, repo_settings)
40+
finally:
41+
os.close(fd)
3942

4043
try:
4144
dynconf_kwargs = {'core_loaders': [], # DISABLE default loaders, otherwise will load toml files more than once.
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
"""Test-only helpers for snapshotting/restoring Dynaconf settings.
2+
3+
Dynaconf's ``settings.unset(key, force=True)`` does not reliably remove a
4+
dotted leaf (e.g. ``"openai.deployment_id"``) after that leaf was created
5+
via ``settings.set(...)``. The leaf survives inside the parent section's
6+
``DynaBox``, which causes state to leak between tests that share the global
7+
settings singleton.
8+
9+
These helpers provide a SENTINEL-based snapshot so that keys which were
10+
originally absent are truly removed (not just rewritten to ``None``) during
11+
restore, including for dotted keys.
12+
"""
13+
14+
from pr_agent.config_loader import get_settings
15+
16+
SENTINEL = object()
17+
18+
19+
def snapshot_settings(keys):
20+
"""Capture current values for ``keys``; missing keys map to ``SENTINEL``."""
21+
settings = get_settings()
22+
return {key: settings.get(key, SENTINEL) for key in keys}
23+
24+
25+
def _remove_key(settings, key):
26+
"""Best-effort removal of ``key`` (supports dotted leaves)."""
27+
if "." in key:
28+
section_name, leaf = key.split(".", 1)
29+
container = getattr(settings, section_name, None)
30+
if container is None:
31+
return
32+
# DynaBox lookups are case-insensitive, but ``pop`` requires the
33+
# stored casing. Find the matching key and pop it.
34+
for stored in list(container.keys()):
35+
if stored.lower() == leaf.lower():
36+
# ``pop`` with a default never raises KeyError; we narrow to
37+
# ``(AttributeError, TypeError)`` to tolerate exotic container
38+
# types that don't implement a dict-like ``pop``. Any other
39+
# exception is unexpected and should surface so tests fail
40+
# loudly rather than mask Dynaconf state leaks.
41+
try:
42+
container.pop(stored, None)
43+
except (AttributeError, TypeError):
44+
pass
45+
return
46+
return
47+
# ``settings.unset`` raises ``KeyError`` for keys that were never set;
48+
# that case is benign for restore. Any other exception (e.g. a Dynaconf
49+
# internal error) must propagate so that a broken cleanup is visible
50+
# instead of silently leaking state across tests.
51+
try:
52+
settings.unset(key, force=True)
53+
except KeyError:
54+
pass
55+
56+
57+
def restore_settings(snapshot):
58+
"""Restore ``snapshot``; truly remove entries whose snapshot is SENTINEL."""
59+
settings = get_settings()
60+
for key, value in snapshot.items():
61+
if value is SENTINEL:
62+
_remove_key(settings, key)
63+
else:
64+
settings.set(key, value)

0 commit comments

Comments
 (0)