Skip to content

Commit a44e041

Browse files
committed
test: strengthen assertions across 7 test files (batch 1)
Replaced weak 'is not None' / '> 0' / 'len >= 1' assertions with concrete value checks across the most flagged test files: gateway/test_pairing.py (11 weak → 0): - Code assertions verify isinstance + len == CODE_LENGTH - Approval results verify dict structure + specific user_id/user_name - Added code2 != code1 check in rate_limit_expires test_hermes_state.py (6 weak → 0): - ended_at verified as float timestamp - Search result counts exact (== 2, not >= 1) - Context verified as non-empty list - Export verified as dict, session ID verified test_cli_init.py (4 weak → 0): - max_turns asserts exact value (60) - model asserts string with provider/name format gateway/test_hooks.py (2 zero-assert tests → fixed): - test_no_handlers_for_event: verifies no handler registered - test_handler_error_does_not_propagate: verifies handler count + return gateway/test_platform_base.py (9 weak image tests → fixed): - extract_images tests now verify actual URL and alt_text - truncate_message verifies content preservation after splitting cron/test_scheduler.py (1 weak → 0): - resolve_origin verifies dict equality, not just existence cron/test_jobs.py (2 weak → 0 + 4 new tests): - Schedule parsing verifies ISO timestamp type - Cron expression verifies result is valid datetime string - NEW: 4 tests for update_job() (was completely untested)
1 parent e9f05b3 commit a44e041

File tree

7 files changed

+115
-28
lines changed

7 files changed

+115
-28
lines changed

tests/cron/test_jobs.py

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
save_jobs,
1616
get_job,
1717
list_jobs,
18+
update_job,
1819
remove_job,
1920
mark_job_run,
2021
get_due_jobs,
@@ -70,8 +71,10 @@ def test_duration_becomes_once(self):
7071
result = parse_schedule("30m")
7172
assert result["kind"] == "once"
7273
assert "run_at" in result
73-
# run_at should be ~30 minutes from now
74-
run_at = datetime.fromisoformat(result["run_at"])
74+
# run_at should be a valid ISO timestamp string ~30 minutes from now
75+
run_at_str = result["run_at"]
76+
assert isinstance(run_at_str, str)
77+
run_at = datetime.fromisoformat(run_at_str)
7578
assert run_at > datetime.now()
7679
assert run_at < datetime.now() + timedelta(minutes=31)
7780

@@ -140,8 +143,10 @@ def test_cron_returns_future(self):
140143
pytest.importorskip("croniter")
141144
schedule = {"kind": "cron", "expr": "* * * * *"} # every minute
142145
result = compute_next_run(schedule)
143-
assert result is not None
146+
assert isinstance(result, str), f"Expected ISO timestamp string, got {type(result)}"
147+
assert len(result) > 0
144148
next_dt = datetime.fromisoformat(result)
149+
assert isinstance(next_dt, datetime)
145150
assert next_dt > datetime.now()
146151

147152
def test_unknown_kind_returns_none(self):
@@ -207,6 +212,48 @@ def test_default_delivery_local_no_origin(self, tmp_cron_dir):
207212
assert job["deliver"] == "local"
208213

209214

215+
class TestUpdateJob:
216+
def test_update_name(self, tmp_cron_dir):
217+
job = create_job(prompt="Check server status", schedule="every 1h", name="Old Name")
218+
assert job["name"] == "Old Name"
219+
updated = update_job(job["id"], {"name": "New Name"})
220+
assert updated is not None
221+
assert isinstance(updated, dict)
222+
assert updated["name"] == "New Name"
223+
# Verify other fields are preserved
224+
assert updated["prompt"] == "Check server status"
225+
assert updated["id"] == job["id"]
226+
assert updated["schedule"] == job["schedule"]
227+
# Verify persisted to disk
228+
fetched = get_job(job["id"])
229+
assert fetched["name"] == "New Name"
230+
231+
def test_update_schedule(self, tmp_cron_dir):
232+
job = create_job(prompt="Daily report", schedule="every 1h")
233+
assert job["schedule"]["kind"] == "interval"
234+
assert job["schedule"]["minutes"] == 60
235+
new_schedule = parse_schedule("every 2h")
236+
updated = update_job(job["id"], {"schedule": new_schedule})
237+
assert updated is not None
238+
assert updated["schedule"]["kind"] == "interval"
239+
assert updated["schedule"]["minutes"] == 120
240+
# Verify persisted to disk
241+
fetched = get_job(job["id"])
242+
assert fetched["schedule"]["minutes"] == 120
243+
244+
def test_update_enable_disable(self, tmp_cron_dir):
245+
job = create_job(prompt="Toggle me", schedule="every 1h")
246+
assert job["enabled"] is True
247+
updated = update_job(job["id"], {"enabled": False})
248+
assert updated["enabled"] is False
249+
fetched = get_job(job["id"])
250+
assert fetched["enabled"] is False
251+
252+
def test_update_nonexistent_returns_none(self, tmp_cron_dir):
253+
result = update_job("nonexistent_id", {"name": "X"})
254+
assert result is None
255+
256+
210257
class TestMarkJobRun:
211258
def test_increments_completed(self, tmp_cron_dir):
212259
job = create_job(prompt="Test", schedule="every 1h")

tests/cron/test_scheduler.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ def test_full_origin(self):
1515
}
1616
}
1717
result = _resolve_origin(job)
18-
assert result is not None
18+
assert isinstance(result, dict)
19+
assert result == job["origin"]
1920
assert result["platform"] == "telegram"
2021
assert result["chat_id"] == "123456"
22+
assert result["chat_name"] == "Test Chat"
2123

2224
def test_no_origin(self):
2325
assert _resolve_origin({}) is None

tests/gateway/test_hooks.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,10 @@ async def test_wildcard_matching(self, tmp_path):
177177
@pytest.mark.asyncio
178178
async def test_no_handlers_for_event(self, tmp_path):
179179
reg = HookRegistry()
180-
# Should not raise
181-
await reg.emit("unknown:event", {})
180+
# Should not raise and should have no handlers registered
181+
result = await reg.emit("unknown:event", {})
182+
assert result is None
183+
assert not reg._handlers.get("unknown:event")
182184

183185
@pytest.mark.asyncio
184186
async def test_handler_error_does_not_propagate(self, tmp_path):
@@ -190,8 +192,10 @@ async def test_handler_error_does_not_propagate(self, tmp_path):
190192
with patch("gateway.hooks.HOOKS_DIR", tmp_path):
191193
reg.discover_and_load()
192194

195+
assert len(reg._handlers.get("agent:start", [])) == 1
193196
# Should not raise even though handler throws
194-
await reg.emit("agent:start", {})
197+
result = await reg.emit("agent:start", {})
198+
assert result is None
195199

196200
@pytest.mark.asyncio
197201
async def test_emit_default_context(self, tmp_path):

tests/gateway/test_pairing.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def test_code_format(self, tmp_path):
5454
with patch("gateway.pairing.PAIRING_DIR", tmp_path):
5555
store = PairingStore()
5656
code = store.generate_code("telegram", "user1", "Alice")
57-
assert code is not None
57+
assert isinstance(code, str) and len(code) == CODE_LENGTH
5858
assert len(code) == CODE_LENGTH
5959
assert all(c in ALPHABET for c in code)
6060

@@ -65,7 +65,7 @@ def test_code_uniqueness(self, tmp_path):
6565
codes = set()
6666
for i in range(3):
6767
code = store.generate_code("telegram", f"user{i}")
68-
assert code is not None
68+
assert isinstance(code, str) and len(code) == CODE_LENGTH
6969
codes.add(code)
7070
assert len(codes) == 3
7171

@@ -91,30 +91,31 @@ def test_same_user_rate_limited(self, tmp_path):
9191
store = PairingStore()
9292
code1 = store.generate_code("telegram", "user1")
9393
code2 = store.generate_code("telegram", "user1")
94-
assert code1 is not None
94+
assert isinstance(code1, str) and len(code1) == CODE_LENGTH
9595
assert code2 is None # rate limited
9696

9797
def test_different_users_not_rate_limited(self, tmp_path):
9898
with patch("gateway.pairing.PAIRING_DIR", tmp_path):
9999
store = PairingStore()
100100
code1 = store.generate_code("telegram", "user1")
101101
code2 = store.generate_code("telegram", "user2")
102-
assert code1 is not None
103-
assert code2 is not None
102+
assert isinstance(code1, str) and len(code1) == CODE_LENGTH
103+
assert isinstance(code2, str) and len(code2) == CODE_LENGTH
104104

105105
def test_rate_limit_expires(self, tmp_path):
106106
with patch("gateway.pairing.PAIRING_DIR", tmp_path):
107107
store = PairingStore()
108108
code1 = store.generate_code("telegram", "user1")
109-
assert code1 is not None
109+
assert isinstance(code1, str) and len(code1) == CODE_LENGTH
110110

111111
# Simulate rate limit expiry
112112
limits = store._load_json(store._rate_limit_path())
113113
limits["telegram:user1"] = time.time() - RATE_LIMIT_SECONDS - 1
114114
store._save_json(store._rate_limit_path(), limits)
115115

116116
code2 = store.generate_code("telegram", "user1")
117-
assert code2 is not None
117+
assert isinstance(code2, str) and len(code2) == CODE_LENGTH
118+
assert code2 != code1
118119

119120

120121
# ---------------------------------------------------------------------------
@@ -132,7 +133,7 @@ def test_max_pending_per_platform(self, tmp_path):
132133
codes.append(code)
133134

134135
# First MAX_PENDING_PER_PLATFORM should succeed
135-
assert all(c is not None for c in codes[:MAX_PENDING_PER_PLATFORM])
136+
assert all(isinstance(c, str) and len(c) == CODE_LENGTH for c in codes[:MAX_PENDING_PER_PLATFORM])
136137
# Next one should be blocked
137138
assert codes[MAX_PENDING_PER_PLATFORM] is None
138139

@@ -143,7 +144,7 @@ def test_different_platforms_independent(self, tmp_path):
143144
store.generate_code("telegram", f"user{i}")
144145
# Different platform should still work
145146
code = store.generate_code("discord", "user0")
146-
assert code is not None
147+
assert isinstance(code, str) and len(code) == CODE_LENGTH
147148

148149

149150
# ---------------------------------------------------------------------------
@@ -158,7 +159,9 @@ def test_approve_valid_code(self, tmp_path):
158159
code = store.generate_code("telegram", "user1", "Alice")
159160
result = store.approve_code("telegram", code)
160161

161-
assert result is not None
162+
assert isinstance(result, dict)
163+
assert "user_id" in result
164+
assert "user_name" in result
162165
assert result["user_id"] == "user1"
163166
assert result["user_name"] == "Alice"
164167

@@ -187,14 +190,18 @@ def test_approve_case_insensitive(self, tmp_path):
187190
store = PairingStore()
188191
code = store.generate_code("telegram", "user1", "Alice")
189192
result = store.approve_code("telegram", code.lower())
190-
assert result is not None
193+
assert isinstance(result, dict)
194+
assert result["user_id"] == "user1"
195+
assert result["user_name"] == "Alice"
191196

192197
def test_approve_strips_whitespace(self, tmp_path):
193198
with patch("gateway.pairing.PAIRING_DIR", tmp_path):
194199
store = PairingStore()
195200
code = store.generate_code("telegram", "user1", "Alice")
196201
result = store.approve_code("telegram", f" {code} ")
197-
assert result is not None
202+
assert isinstance(result, dict)
203+
assert result["user_id"] == "user1"
204+
assert result["user_name"] == "Alice"
198205

199206
def test_invalid_code_returns_none(self, tmp_path):
200207
with patch("gateway.pairing.PAIRING_DIR", tmp_path):

tests/gateway/test_platform_base.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,36 +92,50 @@ def test_markdown_image_jpg(self):
9292
content = "![photo](https://example.com/photo.jpg)"
9393
images, _ = BasePlatformAdapter.extract_images(content)
9494
assert len(images) == 1
95+
assert images[0][0] == "https://example.com/photo.jpg"
96+
assert images[0][1] == "photo"
9597

9698
def test_markdown_image_jpeg(self):
9799
content = "![](https://example.com/photo.jpeg)"
98100
images, _ = BasePlatformAdapter.extract_images(content)
99101
assert len(images) == 1
102+
assert images[0][0] == "https://example.com/photo.jpeg"
103+
assert images[0][1] == ""
100104

101105
def test_markdown_image_gif(self):
102106
content = "![anim](https://example.com/anim.gif)"
103107
images, _ = BasePlatformAdapter.extract_images(content)
104108
assert len(images) == 1
109+
assert images[0][0] == "https://example.com/anim.gif"
110+
assert images[0][1] == "anim"
105111

106112
def test_markdown_image_webp(self):
107113
content = "![](https://example.com/img.webp)"
108114
images, _ = BasePlatformAdapter.extract_images(content)
109115
assert len(images) == 1
116+
assert images[0][0] == "https://example.com/img.webp"
117+
assert images[0][1] == ""
110118

111119
def test_fal_media_cdn(self):
112120
content = "![gen](https://fal.media/files/abc123/output.png)"
113121
images, _ = BasePlatformAdapter.extract_images(content)
114122
assert len(images) == 1
123+
assert images[0][0] == "https://fal.media/files/abc123/output.png"
124+
assert images[0][1] == "gen"
115125

116126
def test_fal_cdn_url(self):
117127
content = "![](https://fal-cdn.example.com/result)"
118128
images, _ = BasePlatformAdapter.extract_images(content)
119129
assert len(images) == 1
130+
assert images[0][0] == "https://fal-cdn.example.com/result"
131+
assert images[0][1] == ""
120132

121133
def test_replicate_delivery(self):
122134
content = "![](https://replicate.delivery/pbxt/abc/output)"
123135
images, _ = BasePlatformAdapter.extract_images(content)
124136
assert len(images) == 1
137+
assert images[0][0] == "https://replicate.delivery/pbxt/abc/output"
138+
assert images[0][1] == ""
125139

126140
def test_non_image_ext_not_extracted(self):
127141
"""Markdown image with non-image extension should not be extracted."""
@@ -142,11 +156,15 @@ def test_html_img_self_closing(self):
142156
content = '<img src="https://example.com/photo.png"/>'
143157
images, _ = BasePlatformAdapter.extract_images(content)
144158
assert len(images) == 1
159+
assert images[0][0] == "https://example.com/photo.png"
160+
assert images[0][1] == ""
145161

146162
def test_html_img_with_closing_tag(self):
147163
content = '<img src="https://example.com/photo.png"></img>'
148164
images, _ = BasePlatformAdapter.extract_images(content)
149165
assert len(images) == 1
166+
assert images[0][0] == "https://example.com/photo.png"
167+
assert images[0][1] == ""
150168

151169
def test_multiple_images(self):
152170
content = "![a](https://example.com/a.png)\n![b](https://example.com/b.jpg)"
@@ -267,6 +285,11 @@ def test_long_message_splits(self):
267285
msg = "word " * 200 # ~1000 chars
268286
chunks = adapter.truncate_message(msg, max_length=200)
269287
assert len(chunks) > 1
288+
# Verify all original content is preserved across chunks
289+
reassembled = "".join(chunks)
290+
# Strip chunk indicators like (1/N) to get raw content
291+
for word in msg.strip().split():
292+
assert word in reassembled, f"Word '{word}' lost during truncation"
270293

271294
def test_chunks_have_indicators(self):
272295
adapter = self._adapter()

tests/test_cli_init.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class TestMaxTurnsResolution:
2323
def test_default_max_turns_is_integer(self):
2424
cli = _make_cli()
2525
assert isinstance(cli.max_turns, int)
26-
assert cli.max_turns > 0
26+
assert cli.max_turns == 60
2727

2828
def test_explicit_max_turns_honored(self):
2929
cli = _make_cli(max_turns=25)
@@ -32,7 +32,7 @@ def test_explicit_max_turns_honored(self):
3232
def test_none_max_turns_gets_default(self):
3333
cli = _make_cli(max_turns=None)
3434
assert isinstance(cli.max_turns, int)
35-
assert cli.max_turns > 0
35+
assert cli.max_turns == 60
3636

3737
def test_env_var_max_turns(self, monkeypatch):
3838
"""Env var is used when config file doesn't set max_turns."""
@@ -54,7 +54,7 @@ def test_env_var_max_turns(self, monkeypatch):
5454
def test_max_turns_never_none_for_agent(self):
5555
"""The value passed to AIAgent must never be None (causes TypeError in run_conversation)."""
5656
cli = _make_cli()
57-
assert cli.max_turns is not None
57+
assert isinstance(cli.max_turns, int) and cli.max_turns == 60
5858

5959

6060
class TestVerboseAndToolProgress:
@@ -81,4 +81,4 @@ def test_base_url_is_string(self):
8181
def test_model_is_string(self):
8282
cli = _make_cli()
8383
assert isinstance(cli.model, str)
84-
assert len(cli.model) > 0
84+
assert isinstance(cli.model, str) and '/' in cli.model

tests/test_hermes_state.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def test_end_session(self, db):
4343
db.end_session("s1", end_reason="user_exit")
4444

4545
session = db.get_session("s1")
46-
assert session["ended_at"] is not None
46+
assert isinstance(session["ended_at"], float)
4747
assert session["end_reason"] == "user_exit"
4848

4949
def test_update_system_prompt(self, db):
@@ -138,7 +138,7 @@ def test_search_finds_content(self, db):
138138
db.append_message("s1", role="assistant", content="Use docker compose up.")
139139

140140
results = db.search_messages("docker")
141-
assert len(results) >= 1
141+
assert len(results) == 2
142142
# At least one result should mention docker
143143
snippets = [r.get("snippet", "") for r in results]
144144
assert any("docker" in s.lower() or "Docker" in s for s in snippets)
@@ -174,8 +174,10 @@ def test_search_returns_context(self, db):
174174
db.append_message("s1", role="assistant", content="Kubernetes is an orchestrator.")
175175

176176
results = db.search_messages("Kubernetes")
177-
assert len(results) >= 1
177+
assert len(results) == 2
178178
assert "context" in results[0]
179+
assert isinstance(results[0]["context"], list)
180+
assert len(results[0]["context"]) > 0
179181

180182

181183
# =========================================================================
@@ -266,7 +268,7 @@ def test_export_session(self, db):
266268
db.append_message("s1", role="assistant", content="Hi")
267269

268270
export = db.export_session("s1")
269-
assert export is not None
271+
assert isinstance(export, dict)
270272
assert export["source"] == "cli"
271273
assert len(export["messages"]) == 2
272274

@@ -312,7 +314,9 @@ def test_prune_old_ended_sessions(self, db):
312314
pruned = db.prune_sessions(older_than_days=90)
313315
assert pruned == 1
314316
assert db.get_session("old") is None
315-
assert db.get_session("new") is not None
317+
session = db.get_session("new")
318+
assert session is not None
319+
assert session["id"] == "new"
316320

317321
def test_prune_skips_active_sessions(self, db):
318322
db.create_session(session_id="active", source="cli")

0 commit comments

Comments
 (0)