fix(logging): prevent test-generated error issues - #109
Conversation
Inject typed error logging into dictation and paste flows so negative-path tests can record or discard expected failures without writing unified-log error entries. Production defaults retain the existing subsystem, categories, severity, and messages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4ea0580f-655c-457e-9b12-dd7e8dc1dfe5
There was a problem hiding this comment.
Copilot review overview
🟢 Approved
No unresolved issues were identified, and production logging behavior is preserved.
Review effort: Lite
Findings: None
What changed in this PR
This pull request adds injectable error logging to prevent intentional test errors from generating false issues while preserving production logging behavior.
Changes:
- Adds an
ErrorLoggingabstraction and unified-log implementation. - Injects loggers into
AppViewModelandPasteManager. - Updates tests to suppress and verify expected error messages.
| File | Description |
|---|---|
Tests/OpenWritrTests/PasteManagerTests.swift |
Suppresses test logs and verifies clipboard restoration errors. |
Tests/OpenWritrTests/AppViewModelDictationFlowTests.swift |
Records and verifies transcription and audio errors. |
Sources/OpenWritr/PasteManager.swift |
Routes paste errors through the injected logger. |
Sources/OpenWritr/OpenWritrApp.swift |
Injects logging into application error paths. |
Sources/OpenWritr/ErrorLogger.swift |
Defines the logging abstraction and production implementation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
macOS UI Review — PR #109
Surfaces reviewed: Settings (light/dark, incl. larger accessibility text), About (light/dark, incl. larger accessibility text), and the overlay in all five states (listening, transcribing, enhancing, done, error) in both light and dark appearances.
Findings: No high-confidence, actionable HIG defects were found.
This PR is a pure logging refactor: it introduces ErrorLogger.swift (an ErrorLogging protocol + UnifiedErrorLogger) and threads an injected errorLogger through AppViewModel and PasteManager, replacing direct appLog.error(...)/pasteLog.error(...) calls. It does not touch any SwiftUI view code, layout, colors, text styles, controls, or accessibility labels for Settings, About, or the overlay. Consequently none of the rendered surfaces show any visible regression attributable to this diff — all states render correctly with proper contrast and legible text at both standard and accessibility text sizes in light and dark mode.
One non-UI observation for awareness only (not reported as a finding since it is not a rendered defect): the new UnifiedErrorLogger.logError marks its entire message string .public in os.log, consistent with the prior per-call privacy: .public usage on error.localizedDescription at the replaced call sites — no privacy regression is evident from the diff.
Summary
This prevents required local
swift testruns from creating false GitHub issues such as #69 while leaving production error logging unchanged.Validation
OS_ACTIVITY_MODE=disable swift build -c release -Xswiftc -warnings-as-errorsOS_ACTIVITY_MODE=disable swiftlint lint --strictOS_ACTIVITY_MODE=disable swift test(38 tests)Notes
Generated with an agent. No dependencies changed. No issues are closed by this PR.