Repository navigation
Conversation
The export read OSLogStore directly, so any test of the fallback path ran against the real unified log and depended on whether the machine lets the process read its own log store. The fallback is now a LogSource value (a name plus a "read entries since" closure returning dated lines) passed into export, with the OSLogStore reader as the default. The log directory and the current time are parameters too, so tests can drive both sources and name today's file from the same instant the export uses. Reading the diagnostic log file is split out into readExcerpt, which returns the kept lines plus how many of them start an entry, and the file tests move to that pure layer. The export writes whatever the file yields, exactly as before; the entry count is unused until the fallback decision needs it. The body is still joined straight from the file's substrings, so a large log is not copied line by line. The @available(macOS 12) annotations are dropped: the package targets macOS 14.
… complete Export Diagnostics read the persistent log file whenever today's file existed. The streamer creates that file before it starts `log stream`, and `log stream` can give up at once (it needs admin rights on some macOS builds), so a user exporting in that state got a header-only file that looked like a successful export, and the OSLogStore fallback was never reached. The file is now the source only when it holds at least one entry inside the export window; otherwise the export comes from the fallback. The fallback's entries are filtered to the window by their own dates: the starting position OSLogStore is asked for is reported to be ignored for the current-process scope on some releases, and since a non-empty fallback now decides what is exported, launch-time entries must not pass as the last thirty minutes. For a user who may not read the log store, the fallback can fail as well: OSLogStore throws there. Rethrowing would turn an export that used to produce a file into an error, and a header-only file cannot be told apart from a full one. Instead the export succeeds and writes the header plus one notice line: "No log entries were found" when both sources answered and were empty, "No recent log entries could be read" when one failed, with each source's part, and for a failure only the error domain and code. The error description is left out because it can carry a file path and with it the user name. The same applies when the file is missing and the fallback throws, which previously failed the export. Writing the export itself is not classified: a failed write throws as before and never sends the export on to the fallback. When the file is unreadable but the fallback has entries, one notice line before them says so, by domain and code, so a reader knows the entries come from the fallback because the file failed. The return value is now the number of entries from whichever source was used, not the number of lines: prelude, continuation lines and the empty line after the final newline are not entries, and Settings decides from it whether the export holds anything recent. The exported log line names it entries accordingly. `log stream` sends its own stdout and stderr into the file, so a refusal to start stands before the first entry. When the file has nothing in the window, those lines are kept in the export next to the notice or the fallback entries, as the best clue to why. The file is decoded lossily. The streamer appends raw pipe chunks, so a read can meet a multi-byte character split across two chunks, and the strict decoder would report the whole file as corrupt. It is mapped, so the bytes do not sit on the heap next to the decoded string. Rejected: rethrowing the fallback's error when the file has nothing recent. It would surface the failure in Settings, but it leaves the user with nothing to attach to a bug report, and the header alone (build, macOS version, settings) is still worth having.
…ntries An export that finds nothing recent now still succeeds and writes the header plus a notice, so Settings showed it exactly like a full export: "Exported to: <file>". Someone attaching that file to a bug report had no reason to look inside first. The line under the button now comes from DiagnosticExportStatus, a pure value built from the export's result: a full export names the file, an export with no entries adds that the file holds no recent log entries and says why (in orange), and a failure keeps the red error text. The decision is tested on the value; one ViewInspector test pins that the status line renders its message. No new window or alert.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #974 +/- ##
=======================================
+ Coverage 91.9% 92.0% +0.1%
=======================================
Files 317 318 +1
Lines 24317 24398 +81
Branches 6 6
=======================================
+ Hits 22356 22464 +108
+ Misses 1957 1930 -27
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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
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.
What
Settings → Advanced → "Export Diagnostics…" could write a file that holds only the header and present it as a successful export.
The export read today's diagnostic log file whenever that file existed, without checking that it had anything inside the requested window. The streamer creates the day's file before
log streamstarts, andlog streamcan give up right away, so a file with nothing recent is a normal state.OSLogStorewas consulted only when the file was missing, and when it threw, the export failed.Now:
OSLogStore, filters those entries to the window itself instead of trusting the store's starting position, and says in the file that the entries come from there.log streamgo into the file, so this is where its own messages land.diagnostics_exportedlog key isentries=accordingly.The fallback source is injectable, so the tests never touch the real
OSLogStore.Known limits, not changed here
OSLogStorewhile today's file existed. An attempt to append the store's newer entries was dropped during review: it filled only a tail, never a gap in the middle, and its duplicate detection had no margin because the file's timestamps parse to milliseconds.log streamoutput after earlier entries. Output thatlog streamwrites after earlier entries, such as a refusal after a relaunch later in the day, carries no timestamp and counts as a continuation of the last entry before it, so it is dropped together with that out-of-window entry; only such output before the file's first entry is kept.Tests
DiagnosticExporterTests(28) cover: file with an entry in the window (fallback not consulted), file without one plus fallback entries, fallback throwing, fallback empty, missing file, unreadable file, invalid UTF-8 between two entries, fallback entries older than the window, prelude kept, unwritable output, entry count. The fake fallback's error description contains a made-up home path and a test asserts it never reaches the file.DiagnosticExportStatusTests(4) cover the status value and its rendering by accessibility identifier.lint.sh,lint.sh --analyze,pre-push.sh --with-appstorepass locally.Not verified: a run as a real non-admin user; the real
OSLogStore(its reader-side date filter has no test of its own); a sandboxed App Store run; the new status line in the running app; that the prelude really carries the refusal, which is read from the code and not from an observed file.AI assistance
Written with Claude (Opus 5.5): the analysis of the current behaviour, the change, the tests and the commit messages. Reviewed and re-verified locally with Claude (Fable 5.2): diff read, three review passes with the scope cut back after the second, full suite, lint, analyze, release parity for both variants, and a mutation of the core decision.
Checklist
swift test --parallel,./scripts/lint.sh,./scripts/lint.sh --analyze./scripts/pre-push.sh --with-appstore