Skip to content

Replace bare assertions with proper error handling; add crypto tests#17

Draft
offbyonebit wants to merge 1 commit into
mainfrom
claude/test-bug-review-5R0n5
Draft

Replace bare assertions with proper error handling; add crypto tests#17
offbyonebit wants to merge 1 commit into
mainfrom
claude/test-bug-review-5R0n5

Conversation

@offbyonebit
Copy link
Copy Markdown
Owner

Summary

  • Bug fix: 5 bare assert statements used as production error guards replaced with explicit if/raise RuntimeError or logged early returns — these were silently no-ops under python -O and would produce unhelpful AssertionError tracebacks otherwise
  • New tests: tests/test_crypto.py adds 18 tests for the encryption module (crypto.py), which had zero dedicated coverage despite being security-critical
  • All 61 tests pass

Changes

Assertion fixes

File Location Old New
main.py:155 after Syncthing start assert client is not None raise RuntimeError(...)
main.py:312 _accept_device() assert client is not None log + early return
syncthing.py:715 _spawn() assert self._binary is not None raise RuntimeError(...)
ui.py:90 _read_events() assert proc.stdout is not None raise RuntimeError(...)
ui.py:1023 _finish_update_check() assert info is not None if info is None: return

New crypto tests cover

  • Encrypt/decrypt round-trip (text, binary, empty, unicode passphrase)
  • Wrong passphrase returns None
  • V1 produces magic header and random salt per call
  • V0 legacy format backward compatibility
  • Truncated, garbage, and partial-magic input
  • is_encrypted() for v0, v1, plaintext, and empty input

Test plan

  • pytest tests/test_crypto.py — 18 new tests pass
  • pytest tests/ (excluding integration) — all 61 tests pass

https://claude.ai/code/session_01BULAPFvuh92QBWZboAuSL2


Generated by Claude Code

Five bare `assert` statements were used as production error guards across
main.py, syncthing.py, and ui.py. These silently become no-ops under
`python -O` and produce unhelpful AssertionError tracebacks otherwise.
Replaced each with an explicit `if ... raise RuntimeError(...)` (fatal
invariant violations) or an early `return` with a logged error (recoverable
caller paths).

Adds tests/test_crypto.py with 18 tests covering encrypt/decrypt
round-trips, wrong-passphrase rejection, v0 legacy format compatibility,
truncated/garbage input, and is_encrypted — the crypto module previously
had no dedicated test coverage.

https://claude.ai/code/session_01BULAPFvuh92QBWZboAuSL2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants