Tests/phase 3#16
Merged
Merged
Conversation
- pyproject: [tool.pytest.ini_options] with testpaths + strict markers (unit/integration/e2e/slow); [tool.coverage.*] with branch + fail_under=83 floor; add pytest-cov to the dev extra - tests/_factories.py: importable shared helpers — make_mbox (the byte-identical builder duplicated across 8 modules), base_config (the shared ingest defaults), make_attachment_message (multipart/attachment builder for attachment tests) - tests/conftest.py: tmp_db, make_account, mbox_factory, config (tmp_path-rooted attachments_dir), mime_factory, and run_cli — a generalized driver for mboxer.cli.main(argv) returning (exit_code, stdout, stderr) Fixtures are added but not yet wired into existing tests; suite stays at 251. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Single-sweep consolidation (suite green before and after, 251 passing): - Replace the byte-identical _make_mbox in 8 modules with the shared make_mbox from _factories (import alias keeps call sites unchanged) - Rebuild the duplicated CONFIG/INGEST_CONFIG bases via base_config(...), removing the hardcoded /tmp attachments_dir smell from every test (test_export now roots ingest attachments under tmp_path explicitly) - Drop now-unused 'import mailbox' (kept in test_ingest, used elsewhere) No behavior change; pure DRY/isolation cleanup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
attachment_output_path() hardcoded idx=0 while extract_attachments() uses a running index, so it computed attachment-0 for every nameless attachment and diverged from where extraction actually writes (attachment-0, attachment-1, ...). Add an idx parameter (default 0, backward-compatible) threaded into the safe filename so callers can predict the correct, distinct path per attachment. Add tests/test_attachments.py (21 tests) covering the corrected nameless-path logic and an end-to-end cross-check against extract_attachments, plus filename sanitization/path-traversal, SHA-256 hashing, duplicate/extensionless names, overlong-name truncation, RFC-2047 decoding, and the empty/pending/error extraction statuses. attachments.py coverage 72% -> 93%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
25 tests driving mboxer.cli.main through the run_cli fixture, asserting exit codes + DB/file effects + key substrings (not stdout dumps): - argparse: no-command/unknown -> exit 2, --help lists subcommands - every subcommand happy path: init-db, account add/list/show/update, ingest, classify, review-categories, security-scan, export notebooklm/jsonl - contract errors: config-not-found (exit 1), account required when multiple exist, unknown account, unknown NotebookLM profile, missing account update, missing category proposal approve/reject - feature branches: --accounts combined export, ingest --create-account, classify --model 'not yet implemented' cli.py coverage 48% -> 94%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite test_limits.py into pure-unit, table-driven tests (25) covering every branch of resolve_notebooklm_limits (default profile, unknown profile, missing/ non-integer keys, all scalar + MB->bytes overrides) and validate_notebooklm_limits (non-positive max_sources, negative reserved, zero budget +/- escape hatch, 200 MB ceiling +/- force, and each soft warning), plus effective_source_budget and a real-config smoke test. limits.py 76% -> 100%. Ratchet coverage floor 83 -> 88 now that the Phase 1 gaps are closed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The historical 'JSONL CLI export profile pass-through' bug lived at the CLI<->exporter seam, not inside either unit (jsonl.py was 98% covered). The JSONL seam is already covered; the NotebookLM exporter, however, was only exercised by direct calls (_do_nlm_export), leaving the cmd_export_notebooklm seam untested. Add CLI-driven tests that run 'mboxer export notebooklm --export-profile ...' and assert the chosen profile reaches the .md output and manifest: metadata-only drops the body (not merely scrubs it), and a raw override surfaces the body that the scrubbed default would have redacted, with the manifest recording the override. A dropped --export-profile in the CLI would now fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ingest.py 85% -> 95%. Adds, reusing the existing config/db_with_account fixtures and mbox builder: - empty mbox completes cleanly (zero counts, run completed) - valid-checkpoint resume continues without loss or duplicates (the skip-to-key path, distinct from the existing invalid-checkpoint restart test) - corrupt mbox (keys() raises) -> run marked failed + 'Failed to open MBOX' - unreadable message (get_message raises) -> ingest_errors row, run still completes - store_body_html + max_body_chars truncation with per-message checkpointing - storage failure mid-message (_store_labels raises) -> recorded as ingest error - KeyboardInterrupt -> run marked 'interrupted' with checkpoint saved for --resume Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
normalize.py 90% -> 99%. Adds: RFC-2047 encoded subject decoding (incl. an unknown-charset header that must fall back, not crash); garbage Date -> date_utc None while date_header is preserved; multipart text/html-only -> stripped-text fallback; empty plain part and non-text content type -> no body; and an unknown part charset -> latin-1 fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…atrix Adds a 6-cell parametrized matrix over thread action (assign=1.0 / assign_hint=0.75) x pre-existing message classification (none / rule_hint 0.75 / rule 1.0), asserting for each cell whether the thread classification is inherited or the explicit message rule is preserved, plus the inherited confidence. Covers the previously under-tested assign_hint thread paths and the equal-confidence (0.75 >= 0.75) preserve boundary. Adds robustness cells: a matching rule with no category_path is a graceful no-op at both thread and message level. classify.py 85% -> 87%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n + lineage Replace the shallow combined --accounts smoke with an end-to-end test that gives each account distinct, identifiable content and asserts: no cross-account bleed (ALPHA content only under alpha/, BETA only under beta/), each account gets its own manifest whose every row carries the correct account_key, and the manifest lineage structure is present (tool_name/export_kind/account_key/source_pack) - checking field presence, not timestamp values. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_resolve_storage_path split the filename with rpartition('.'), which puts a
no-dot name entirely in the 'ext' slot, so the 'if not ext' guard never fired
and a duplicate 'Makefile' resolved to '-1.Makefile'. Key the guard off the
separator instead, so an extensionless duplicate becomes 'Makefile-1' (names
with extensions are unchanged). Update the test to assert the corrected suffix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ixtures A single @pytest.mark.e2e test drives the real CLI end to end (init-db -> account -> ingest a fixed in-code corpus -> classify -> security-scan -> export notebooklm + jsonl) and compares all exported artifacts against a committed golden. Volatile fields are normalized before comparison (tmp_path, ISO/SQLite timestamps, SHA-256 digests, and the setuptools-scm tool version), so the golden is stable across machines, time, and commits. Re-bless intentional changes with MBOXER_BLESS_GOLDEN=1. Kept out of the -m unit inner loop. The golden captures real wiring: USPS->postal (assign, metadata-only body drop), statement->household/utilities (assign_hint, scrubbed), personal->unclassified with its phone number redacted. Promote cli_config/ready fixtures from test_cli.py to conftest.py so both the CLI tests and the e2e test share them (no duplication). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dary Adds property tests over naming.slugify, naming.normalize_category_path, and attachments._safe_attachment_filename with adversarial input (unicode, control chars, path separators, '..' sequences, encoded-word tokens). Invariants assert the security boundary, not just 'doesn't crash': output stays within the allowed charset / single path component, can never escape a base directory, is non-empty, bounded in length, free of control/null bytes, and (for slug) idempotent. Bounded to 300 examples with no deadline so -m unit stays fast (0.65s). Adds hypothesis + pytest-randomly to the dev extra and ignores .hypothesis/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pytest-randomly auto-randomizes every run, but our addopts '-q' hides the
'Using --randomly-seed=...' banner, which would make an order-dependent failure
non-reproducible. Default to deterministic order ('-p no:randomly') so local runs
and the golden are stable and reproducible, and opt into random order explicitly
with 'pytest -p randomly' (the seed is then printed for reproduction).
Verified order-independence: full suite green under seeds 1, 42, 1337, 90210,
8675309, 271828 (349 passed each) — no shared state or leaked isolation found.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the existing CI test job (.github/workflows/ci.yml, Python 3.11 + 3.12)
to run with coverage: 'pytest --cov=mboxer --cov-report=term-missing', which
FAILS the build under the coverage floor in pyproject ([tool.coverage.report]
fail_under) — so coverage can't silently regress on merge (single source of truth).
Add a separate 'random-order' job that runs the suite under pytest-randomly with
the seed printed ('Using --randomly-seed=...') for reproduction. It is an
informational canary (continue-on-error) so it surfaces any future
order-dependence without blocking merges on a non-deterministic signal.
These are test gates only — no change to the release/publish pipeline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The permanent, CI-enforced gate. Coverage cannot regress below 94% on merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Markers and the fast inner loop vs full suite, coverage floor, reproducing a random-order failure by seed, re-blessing the golden, and the shared fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bandit found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing over-haul