Added Some Tests

[henry0816191]

Summary

added/updated some test files

Test plan

• ./run check (or make check)
• pre-commit run --all-files

Related issues

close #14

Summary by CodeRabbit

• Tests

  • Broadened test coverage across DB init, health endpoint, message queue (including throttling/retry), PDF extraction, models, snapshot diffing, sources parsing, and storage behaviors.
  • Added tests for failure scenarios: commit/rollback handling, malformed cached data, raw watchlist seeding, threaded messaging, and version metadata handling.

• Chores

  • Improved test infrastructure with a richer in-memory connection simulation and new seeding helpers to exercise edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR expands test infrastructure and coverage: FakePool gains commit/rollback hooks and seeding helpers; multiple new and extended test modules are added to exercise DB init, health flags, versioning, PDF extraction, message-queue retries/throttling, model parsing, diffing, sources, scout helpers, and storage edge cases.

Changes

Test infra + coverage expansion

Layer / File(s)	Summary
Test harness / data shape tests/conftest.py	FakePool adds fail_on_commit: bool and rollback_count: int; getconn() passes the pool into _FakeConn; _FakeConn.commit() can raise a one-time simulated RuntimeError when fail_on_commit is set; _FakeConn.rollback() marks rollback_called and increments pool.rollback_count; seeding helpers seed_watchlist_raw(...) and seed_paper_cache_invalid_json() added.
Core test modules (new) tests/test_db.py, tests/test_init.py, tests/test_pdf_fetch_unit.py, tests/test_message_queue.py	New modules added: DB pool/init behavior and DDL execution tests; package version resolution tests with autouse reload; in-memory PDF extraction unit test using mocked stream; MessageQueue tests covering send-with-retry, 429 handling, default delays, non-retry errors, throttling, and background-thread enqueueing.
Existing test extensions / edge cases tests/test_health.py, tests/test_models.py, tests/test_monitor.py, tests/test_scout_extra.py, tests/test_sources.py, tests/test_storage.py	Extended tests: health endpoint reflects config flag; Paper ID parsing param tests; diff_snapshots single-field mutation tests; scout notify/channel and batching boundary tests; sources tests change PDF assertions to skip-on-empty, add HEAD retry/backoff test, and HTML four-cell subgroup parsing; storage tests for invalid JSON cache (warning + None), rollback on commit failure, and raw watchlist seeding/matching edge cases.
Wiring / integration assertions tests/test_db.py, tests/test_storage.py, tests/test_message_queue.py	Tests assert pool constructor args (min/max), that init_db executes expected DDL and calls conn.commit() and pool.putconn(conn) even on execute exceptions; storage tests assert rollback count increments on forced commit failure; message-queue threaded test asserts background chat_postMessage() invocation.
Tests / small helpers tests/* (various new/updated test files)	Added helpers and parametrized cases across tests (e.g., _slack_error, _make_stream_cm, autouse reload fixture, parametrized inputs) and updated some live integration tests to skip when extraction is empty.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• cppalliance/paperscout#9: Modifies tests/conftest.py and related test fixture behavior, overlapping FakePool/_FakeConn test-support changes.

Suggested reviewers

• wpak-ai

"I nibble on fixtures, hop through the suite,
Seeded watchlists tucked under a root.
Simulated rolls, retries that gleam,
Tests stitched together — a rabbit's dream. 🐇"

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.80% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Added Some Tests' is vague and generic, using non-descriptive language that fails to convey meaningful information about the specific test changes.	Replace with a more specific title describing the main test coverage added, such as 'Add comprehensive test coverage for database, health, models, and storage modules' or 'Add test suite for connection pooling, message queue, and PDF fetch functionality'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description follows the required template structure with Summary, Test plan, and Related issues sections, though the summary itself is brief.
Linked Issues check	✅ Passed	The PR adds extensive test coverage across multiple modules (database, health, models, storage, scout, sources, etc.) which aligns with issue #14's objective to evaluate and improve test coverage.
Out of Scope Changes check	✅ Passed	All changes are test files (tests/conftest.py, tests/test_db.py, tests/test_health.py, tests/test_init.py, tests/test_message_queue.py, tests/test_models.py, tests/test_monitor.py, tests/test_pdf_fetch_unit.py, tests/test_scout_extra.py, tests/test_sources.py, tests/test_storage.py) directly supporting the test coverage evaluation objective.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_db.py (1)

62-65: ⚡ Quick win

Add a guard that commit() is not called on DDL failure.

Line 65 validates connection return, but without asserting commit was skipped, a regression in error handling could still pass this test.

Suggested test tightening

 def test_init_db_putconn_even_when_execute_fails():
@@
     with pytest.raises(RuntimeError, match="DDL failed"):
         init_db(pool)
 
+    conn.commit.assert_not_called()
     pool.putconn.assert_called_once_with(conn)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_db.py` around lines 62 - 65, The test should also assert that no
commit was performed when init_db(pool) raises a RuntimeError; after calling
init_db(pool) within pytest.raises and before/after
pool.putconn.assert_called_once_with(conn), add an assertion that conn.commit
was not called (use conn.commit.assert_not_called()) so the test verifies commit
is skipped on DDL failure (referencing init_db, conn, and
pool.putconn.assert_called_once_with(conn)).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_db.py`:
- Around line 62-65: The test should also assert that no commit was performed
when init_db(pool) raises a RuntimeError; after calling init_db(pool) within
pytest.raises and before/after pool.putconn.assert_called_once_with(conn), add
an assertion that conn.commit was not called (use
conn.commit.assert_not_called()) so the test verifies commit is skipped on DDL
failure (referencing init_db, conn, and
pool.putconn.assert_called_once_with(conn)).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 747c753e-c638-41b9-b3d7-26f3b69f3547

📥 Commits

Reviewing files that changed from the base of the PR and between 3430468 and 3436550.

📒 Files selected for processing (4)

• tests/conftest.py
• tests/test_db.py
• tests/test_scout_extra.py
• tests/test_storage.py

✅ Files skipped from review due to trivial changes (1)

• tests/test_scout_extra.py

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/test_storage.py
• tests/conftest.py