mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-09-11 00:18:02 +00:00
fix/map-script-error-exits
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Sean WhalenandClaude Opus 5
|
479249040d |
Fix missing-file error paths falling through in find_unknown_base_reverse_dns.py
Two `if not os.path.exists(path): print(f"Error: ...")` checks printed the intended error message but had no sys.exit(1) after it, so execution fell through into the subsequent open() call on the same missing path and raised an unhandled FileNotFoundError instead of the clean error exit the code clearly intended. Sibling duplicate-entry checks in the same function already did print-then-sys.exit(1); these two now match. Sites fixed (both in _main()): - the nested load_list() helper's missing-file check (used for known_unknown_base_reverse_dns.txt and psl_overrides.txt) - the base_reverse_dns_map.csv missing-file check Both sites now have regression tests in a new TestFindUnknownBaseReverseDNS class: - the load_list() site: calling _main() in a temp directory with no known_unknown_base_reverse_dns.txt now raises SystemExit(1) instead of FileNotFoundError. - the base_reverse_dns_map.csv site: _load_as_name_index() does its external work entirely through maxminddb.open_database(), the actual SDK boundary, so that call is mocked to a context manager over an empty iterable rather than loading the real ~23MB bundled MMDB or mocking an internal helper. Calling _main() in a temp directory with no base_reverse_dns_map.csv now raises SystemExit(1) instead of FileNotFoundError. Both tests capture stdout and assert on the specific "Error: ... does not exist" message, pinning the exit to the intended site rather than any sys.exit(1) in the function. Cleanup uses two separate addCleanup calls (rmtree registered before chdir, so LIFO order runs chdir first) instead of one lambda wrapping both, so rmtree still runs even if chdir were to raise. Also fixed an adjacent prose bug a few lines from the second site: the "is in known_unknown... and base_reverse_dns_map..." error message was missing a space after "Error:" and was a backslash-continued f-string that embedded the source's literal indentation in the printed output. It now prints as a single clean line, consistent with the file's other error messages. Logged the user-facing symptom (clean error replaced by a FileNotFoundError traceback) under CHANGELOG.md's Unreleased/Bug fixes section, matching the project's precedent of logging maintainer-tooling fixes (e.g. the sortlists.py entry). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Sean WhalenandClaude Opus 4.7
|
5b08627eaa |
Split tests.py into per-module tests/test_<module>.py (#774)
* Split tests.py into per-module tests/test_<module>.py The 5174-line tests.py monolith is split into per-module files under tests/, mirroring the checkdmarc layout: tests/test_init.py parsedmarc/__init__.py parsing surface tests/test_cli.py parsedmarc/cli.py + config / env-vars / SIGHUP tests/test_utils.py parsedmarc/utils.py (DNS, IP info, PSL, etc.) tests/test_webhook.py parsedmarc/webhook.py tests/test_kafkaclient.py parsedmarc/kafkaclient.py tests/test_splunk.py parsedmarc/splunk.py tests/test_syslog.py parsedmarc/syslog.py tests/test_loganalytics.py parsedmarc/loganalytics.py tests/test_gelf.py parsedmarc/gelf.py tests/test_s3.py parsedmarc/s3.py tests/test_maps.py parsedmarc/resources/maps/ maintainer scripts The split is purely a redistribution — no test bodies changed, no tests added or removed. All 276 existing tests pass under the new layout. The current tests.py contains two kitchen-sink classes (`Test` at line 54 and `TestEnvVarConfig` at line 2360) holding tests that span many modules. Their methods are routed to the correct per-module file by name prefix; the wholly-thematic classes (TestExtractReport, TestUtilsXxx, TestSighupReload, etc.) move whole. Each target file gets its own `class Test(unittest.TestCase)` for the redistributed kitchen-sink methods, plus the thematic classes verbatim. Wiring updates: - `.github/workflows/python-tests.yml`: `pytest ... tests.py` → `python -m pytest ... tests/` (also switches to `python -m pytest` per the checkdmarc convention so cwd lands on the project root). - `pyproject.toml`: adds `[tool.pytest.ini_options] testpaths = ["tests"]` and `[tool.coverage.run] source = ["parsedmarc"]` with an `omit` for `parsedmarc/resources/maps/*.py`. The maps scripts are maintainer-only batch tooling that ships out of the wheel; excluding them from coverage makes the headline number reflect only installed library code. Runtime coverage on the new layout is 59% (was 45% with maps counted), and PR-B will push it to 90%+. - `AGENTS.md`: documents the new layout and how to run individual files / tests; tells future contributors not to reintroduce a monolithic tests.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore 66.9% coverage baseline (count tests/ + parsedmarc) Master's headline 66.9% number on Codecov includes the tests.py file itself (99.35% covered) being measured alongside parsedmarc/*. The original tests.py had no `[tool.coverage.run]` block, so coverage's default — "measure every file imported during the run" — counted the test code as if it were product code. The split commit added `source = ["parsedmarc"]` which suppressed measurement of the test files (correct in principle, since test files aren't shipped code), and that alone made the headline number drop by ~8 percentage points without any actual loss of testing. This commit swaps `source` for an explicit `include = ["parsedmarc/*", "tests/*"]` so both halves are measured the way they were on master. Verified: 276 tests, 66.96% line coverage (effectively unchanged from master's 66.90%). If you want the shipped-code-only number (was the headline that this commit overrides), run `pytest --cov=parsedmarc tests/`. That number is currently 59% and is the focus of the upcoming coverage-expansion PR. Also adds junit.xml to .gitignore so the CI artefact doesn't get accidentally committed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restrict coverage to shipped code (`source = ["parsedmarc"]`) Reverts the prior commit's `include = ["tests/*"]`. Counting the test files toward coverage was wrong — it conflates "shipped code exercised by tests" with "test code that pytest auto-runs", inflates the headline number, and rewards writing more tests rather than tests that verify more code. Master's apparent 66.9% was an artefact of the old monolithic tests.py having no [tool.coverage.run] block at all; coverage's default behaviour measured every imported file, including the test file itself at ~99% "covered", which added ~8 percentage points to the displayed number without any real testing signal. Restricting to `source = ["parsedmarc"]` plus the existing maps omit gives a meaningful baseline: 59% of shipped code is exercised by the test suite today. That's the number the next PR is targeting to lift to 90%+ before the 10.0.0 release; the Codecov "drop" here is a measurement correction, not a regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |