Skip to content

Commit b6df24f

Browse files
committed
Resolve SonarCloud PR-17 findings
- S2583 (test_pdf_download / test_pipeline concurrency tests): the 'condition always false' on 'assert 1 <= peak <= 3' was a static-analysis false positive — Sonar can't see that 'peak' (a nonlocal int) is mutated by the monkeypatched async double via download_pdfs/enrich_collection's gather. Switch the counter to a dict so the mutation isn't const-propagated to 0; same fix applied to the enrich test pre-emptively. - S7503 (test_cli min-citations test): the fake_run_search / fake_shutdown doubles replace awaited production fns and must stay async — mark NOSONAR.
1 parent 6ba99b2 commit b6df24f

3 files changed

Lines changed: 16 additions & 16 deletions

File tree

tests/test_cli.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,11 @@ def test_cli_min_citations_flows_into_query(tmp_path, monkeypatch, sample_papers
325325
previously unreachable from the CLI)."""
326326
captured: dict[str, Query] = {}
327327

328-
async def fake_run_search(query: Query, **_kwargs) -> PaperCollection:
328+
async def fake_run_search(query: Query, **_kwargs) -> PaperCollection: # NOSONAR async stub
329329
captured["query"] = query
330330
return PaperCollection(query=query, papers=tuple(sample_papers))
331331

332-
async def fake_shutdown() -> None:
332+
async def fake_shutdown() -> None: # NOSONAR async stub
333333
return None
334334

335335
monkeypatch.setattr(cli_module, "run_search", fake_run_search)

tests/test_pdf_download.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -347,15 +347,15 @@ async def test_download_pdfs_caps_concurrency(tmp_path: Path, monkeypatch):
347347
hammer a single publisher CDN with one request per paper at once."""
348348
import asyncio
349349

350-
active = 0
351-
peak = 0
350+
# dict (not nonlocal ints) so the counter mutation is visible to static
351+
# analysers that don't trace the monkeypatched async call path.
352+
counters = {"active": 0, "peak": 0}
352353

353354
async def fake_download_one(paper, pdf_dir):
354-
nonlocal active, peak
355-
active += 1
356-
peak = max(peak, active)
355+
counters["active"] += 1
356+
counters["peak"] = max(counters["peak"], counters["active"])
357357
await asyncio.sleep(0.01)
358-
active -= 1
358+
counters["active"] -= 1
359359
return pdf_download_module.PdfDownloadResult(
360360
paper_key=paper.source_id, path=None, skipped_reason=None
361361
)
@@ -364,4 +364,4 @@ async def fake_download_one(paper, pdf_dir):
364364
papers = [_paper(source_id=str(i), pdf_url="https://e/p.pdf") for i in range(8)]
365365
results = await download_pdfs(_collection(*papers), tmp_path, concurrency=3)
366366
assert len(results) == 8
367-
assert 1 <= peak <= 3 # never more than the cap in flight at once
367+
assert 1 <= counters["peak"] <= 3 # never more than the cap in flight at once

tests/test_pipeline.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -326,15 +326,15 @@ async def test_enrich_collection_caps_concurrency(monkeypatch):
326326

327327
from thesisagents.core.models import PaperCollection
328328

329-
active = 0
330-
peak = 0
329+
# dict (not nonlocal ints) so the counter mutation is visible to static
330+
# analysers that don't trace the monkeypatched async call path.
331+
counters = {"active": 0, "peak": 0}
331332

332333
async def fake_enrich_one(paper, *, language, model):
333-
nonlocal active, peak
334-
active += 1
335-
peak = max(peak, active)
334+
counters["active"] += 1
335+
counters["peak"] = max(counters["peak"], counters["active"])
336336
await asyncio.sleep(0.01)
337-
active -= 1
337+
counters["active"] -= 1
338338
return paper
339339

340340
monkeypatch.setattr(pipeline_module, "_enrich_one", fake_enrich_one)
@@ -345,4 +345,4 @@ async def fake_enrich_one(paper, *, language, model):
345345
)
346346
out = await pipeline_module.enrich_collection(collection, concurrency=3)
347347
assert len(out.papers) == 8
348-
assert 1 <= peak <= 3 # never more than the cap in flight at once
348+
assert 1 <= counters["peak"] <= 3 # never more than the cap in flight at once

0 commit comments

Comments
 (0)