From 4fb9bf6ff3704c3ae106359630336b5cbd236ac3 Mon Sep 17 00:00:00 2001 From: Mikhail Yenuchenko Date: Fri, 7 Aug 2026 03:58:49 +0300 Subject: [PATCH] docs: reconcile the documentation with the code An audit of every document against the source turned up drift that had accumulated since the 1.0.2 passes. The screenshot paths README and DEVELOPMENT still point at are deliberately left alone - the images move again when they are retaken. ARCHITECTURE: the jobs_view.go split is six files, not five, since extracting jobs_view_state.go was never counted; the statistics table lists TimedRunCount, which the AvgDurationMS formula already referenced; the store edge of the diagram names methods that exist (LoadJobs and LoadConfig never did); and startup says that Service.Start is called from newMainView rather than from Run. TESTS: three tests had no entry, the latter two being regression tests for 1.0.2 fixes: TestLoadOrCreateConfigPreservesZeroRetentionLimits TestWriteJSONReplacesFileAtomically TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension The deliberately-uncovered list now covers everything the profile actually reports at 0%, so the next redundancy pass does not flag Config, Paths, SaveJobs, or the isEvent markers as gaps. The coverage section measures through -coverprofile and says outright that the per-package percentages -coverpkg prints are not the total - 2.6/8.9/25.5/1.0/61.5 against a real 84.1%. ROADMAP: the over-the-guideline table was re-measured (service.go is over it too now, making six), with a note to re-measure rather than trust it. Co-Authored-By: Claude Opus 5 --- assets/assets.go | 2 +- docs/ARCHITECTURE.md | 16 ++++++++++------ docs/CHANGELOG.md | 11 +++++++++++ docs/ROADMAP.md | 27 +++++++++++++++------------ docs/TESTS.md | 38 +++++++++++++++++++++++++++++--------- 5 files changed, 66 insertions(+), 28 deletions(-) diff --git a/assets/assets.go b/assets/assets.go index 095c2ea..fd67300 100644 --- a/assets/assets.go +++ b/assets/assets.go @@ -17,7 +17,7 @@ import ( // The hard constraint: Fyne's a.SetIcon and SetSystemTrayIcon each take ONE // image, which the OS then scales to every size it needs — titlebar (~16px), // taskbar/dock (~32-48px), and tray. Neither source survives that scaling: -// downscaling the 1254px gosentry-icon-big.png to 16px is muddy, and upscaling +// downscaling the 1254px gosentry-icon-large.png to 16px is muddy, and upscaling // the 16px icon to 32px is blurry. The fix is to feed each surface a // size-appropriate source — which differs per platform because each platform // exposes different icon channels. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4173d41..e910495 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -41,7 +41,7 @@ flowchart LR user -->|"edits jobs, settings, runs commands"| ui ui -->|"CreateJob, UpdateJob, DeleteJob, RunNow, UpdateSettings, AutostartStatus, …"| svc - svc -->|"SaveJobs, SaveConfig, LoadJobs, LoadConfig"| store + svc -->|"OpenStore, PrepareSaveJobs, PrepareSaveConfig, LoadJobsFile"| store store -->|"read/write"| config store -->|"read/write"| jobs @@ -119,10 +119,13 @@ example window-maximized detection, which would need per-OS native calls). ## Main Flows 1. Startup: - `cmd/gosentry` calls `ui.Run`, which creates an `app.Service`, opens the - store, loads `gosentry.json` and `jobs.json`, subscribes the UI to service - events, builds the main window, and calls `Service.Start` to begin the - scheduler loop. On every launch the service seeds per-job run-time statistics + `cmd/gosentry` calls `ui.Run`, which owns the process lifecycle: it calls + `app.Open()` to open the store, load `gosentry.json` and `jobs.json`, and + build the `app.Service`, then hands that Service to `newMainView` + (`mainwindow.go`), which subscribes the UI to service events and calls + `Service.Start` to begin the scheduler loop before assembling the tabs. + `Run` shows the window and, on quit, calls `Service.Stop`. + On every launch the service seeds per-job run-time statistics from existing log files so the details panel reflects accumulated history immediately (see §Statistics below). @@ -239,6 +242,7 @@ applies to them — and so measure launch latency only. | `LastDurationMS` | wall-clock time of the most recent run (launch latency for `StartOnly`) | | `AvgDurationMS` | mean over all runs with a recorded duration, computed as `DurationSumMS / TimedRunCount` on every update rather than folded incrementally, so it never disagrees with the exact sum/count average `runner.aggregateLogStats` computes when seeding from logs | | `MaxDurationMS` | longest recorded run | +| `TimedRunCount` | runs that carried a duration, and so contributed to the aggregates above; a legacy log without a `duration` header counts toward `RunCount` but not this | | `DurationSumMS` | running total of every timed run's duration; the source `AvgDurationMS` is divided from | `runner.RunJob` measures the wall-clock start→finish and sets `DurationMS` on @@ -271,7 +275,7 @@ the moment the window opens. ### `jobs_view.go` file structure The size guideline for a file in this project is ~250 lines. -`src/ui/jobs_view.go` is split across five files along these seams: +`src/ui/jobs_view.go` is split across six files along these seams: | File | Contents | |------|----------| diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b2c6e0f..8070155 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -74,6 +74,17 @@ the app icon (experimental).** - **`docs/ARCHITECTURE.md`** — new §Platform layer: why autostart, file manager, shell, and winproc are OS-specific; compile-time vs runtime branching; rules for adding platform code. +- Documentation audited against the code. `ARCHITECTURE.md` — the `jobs_view.go` + split is six files, not five (the state extraction was never counted), the + statistics table lists `TimedRunCount`, the store edge of the diagram names + the methods that exist, and startup says where `Service.Start` is actually + called. `TESTS.md` — three tests that had no entry are described + (`TestLoadOrCreateConfigPreservesZeroRetentionLimits`, + `TestWriteJSONReplacesFileAtomically`, + `TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension`), the + deliberately-uncovered list covers everything the profile reports at 0%, and + the coverage figure records how to read the total rather than the per-package + lines. `ROADMAP.md` — the over-the-guideline table was re-measured. **Internal:** diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index db038f4..e79f90c 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -145,22 +145,23 @@ Design notes / open questions: and records the `jobs_view.go` and `settings_view.go` splits as the worked examples. `jobs_view.go` was split again in 1.0.2 — into view, state, list, and toolbar — because the selection defect it carried was a symptom of the size -(one 330-line constructor over seven shared locals). Five non-test files are -over the guideline as of that pass: +(one 330-line constructor over seven shared locals). Six non-test files are +over the guideline: | File | Lines | |------|-------| | `src/app/operations.go` | 529 | -| `src/ui/history_view.go` | 373 | -| `src/storage/store.go` | 365 | -| `src/ui/settings_view.go` | 318 | -| `src/app/run.go` | 274 | +| `src/storage/store.go` | 382 | +| `src/ui/history_view.go` | 355 | +| `src/ui/settings_view.go` | 326 | +| `src/app/run.go` | 275 | +| `src/app/service.go` | 252 | -The remaining five are deliberately deferred rather than done piecemeal: a +The remaining six are deliberately deferred rather than done piecemeal: a split touches every reader of the file, and doing them in one pass keeps the -seams consistent instead of settling them five different ways. Splitting is +seams consistent instead of settling each one its own way. Splitting is also the kind of change that reads as pure movement while quietly dropping a -function, so it wants one careful pass, not five hurried ones. +function, so it wants one careful pass, not a hurried one per file. Seams visible today, as a starting point rather than a decision: @@ -174,9 +175,11 @@ Seams visible today, as a starting point rather than a decision: table they size. - **`store.go`** — path resolution, the config load/normalize path, and the jobs load/normalize path are three separate concerns in one file. -- **`run.go`**, **`settings_view.go`** — barely over. Worth re-measuring at the - time; if a pass elsewhere has shrunk them, leave them alone rather than - splitting for the sake of the number. +- **`run.go`**, **`settings_view.go`**, **`service.go`** — barely over. Worth + re-measuring at the time; if a pass elsewhere has shrunk them, leave them + alone rather than splitting for the sake of the number. The counts above move + a few lines either way with any edit, so re-measure before acting on them + rather than treating the table as current. The `jobs_view.go` pass is the worked example for the rest: the constructor was broken up along the state it shared, not along line count, and the split landed diff --git a/docs/TESTS.md b/docs/TESTS.md index 225ab48..dde6e6b 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -66,20 +66,31 @@ exercised from another one's tests — `domain.NewRuntime`, for instance, is covered by the `app` tests. Measure the engine packages together instead: ```bash -go test -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app +go test -coverprofile=cover.out -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app ``` In the PowerShell environment DEVELOPMENT.md prescribes on Windows, PowerShell splits the comma-separated `-coverpkg` list on its own and the command fails with `directory not found`. Use the stop-parsing token, or quote the whole -flag: +flag — and note that `--%` swallows the rest of the line, so the profile has to +be read by a second command: ```powershell -go test --% -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app +go test --% -coverprofile=cover.out -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app ``` -That figure was 84.4% at the 2026-08-04 review, which is the number to compare -against before concluding that coverage has slipped. +The total is the last line of the profile summary. It is **not** any of the +per-package lines `go test` prints: with `-coverpkg` spanning five packages, +each of those reports only what that one package's tests reached across the +whole set, so all five are far below the real figure. + +```powershell +go tool cover -func=cover.out | Select-Object -Last 1 +``` + +That total was 84.4% at the 2026-08-04 review and 84.1% at the 2026-08-07 +documentation audit — the number to compare against before concluding that +coverage has slipped. --- @@ -238,8 +249,8 @@ Tests display-formatting helpers used by the UI. | `TestStatusText` | Verifies that job status codes map to the correct display strings. | | `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. | | `TestEventLine` | Verifies the one-line activity rendering of a `RunRecord`, including the log basename and the `Unknown` fallback for a blank trigger. | -| `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". | -| `TestDisplayArguments` | Verifies that an empty arguments string shows "None". | +| `TestDisplayFolder` | Verifies that an empty folder string shows "(No folder)". | +| `TestDisplayArguments` | Verifies that an empty arguments string shows "(none)". | | `TestDisplayRunMode` | Verifies run-mode labels for normal and start-only modes. | | `TestDisplayInvocation` | Verifies that the full invocation display string combines command and arguments with spacing. | | `TestDisplayIndex` | Verifies the list position of a job index in a filtered index slice. | @@ -265,12 +276,14 @@ Tests JSON round-tripping, default generation, and backward compatibility. | `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults. | | `TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun` | Verifies that a missing jobs file is created with the sample jobs from `defaultJobs`. | | `TestLoadOrCreateConfigKeepsZeroTimeoutOnReload` | Verifies that `default_timeout_seconds: 0` survives a reload rather than being normalized away — 0 is a value, not a missing field. | +| `TestLoadOrCreateConfigPreservesZeroRetentionLimits` | Verifies that `max_log_files` / `max_log_age_days` of 0 read back as 0 ("keep everything") instead of being backfilled to the 100 / 30 defaults — a field the file sets is not the missing-field case. | | `TestLoadOrCreateConfigMigratesJobsDir` | Verifies that a pre-0.15 `jobs_dir` becomes `jobs_file` pointing at the same `jobs.json`, and that the retired key is not written back. | | `TestLoadOrCreateConfigMigratesLegacyThemeDefault` | Verifies that a config storing the retired `"default"` theme value is normalized to `system` on load. | | `TestLoadJobsFileReportsMissingWithoutCreating` | Verifies that `LoadJobsFile` reports a missing file as not-found without creating or seeding it, and normalizes the jobs it does load. | | `TestApplyConfigPathsDerivesJobsDir` | Verifies that the configured jobs file resolves against the program folder and that `Paths.JobsDir` is derived from it. | | `TestJobTimeoutRoundTripsThreeStates` | Verifies the on-disk encoding that keeps "inherit" and "no timeout" distinguishable: `nil` is omitted entirely, an explicit `0` is written and read back as set. | | `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. | +| `TestWriteJSONReplacesFileAtomically` | Pins the durability fix: `writeJSON` replaces the destination through a temp file and a rename rather than truncating it in place, and leaves no temp file behind. | --- @@ -355,6 +368,7 @@ Tests the Windows shell invocation and hidden-window flags. | `TestShellCommandHidesWindow` | Verifies that shell commands request hidden-window startup to prevent console flash. | | `TestShellCommandUsesWindowsSafeQuoting` | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. | | `TestWindowsShellCommandLineQuotesUnquotedProgramPath` | Verifies that unquoted program paths in shell commands are quoted while preserving already-quoted arguments. | +| `TestQuoteLeadingWindowsProgramPathPicksEarliestBoundedExtension` | Regression: the program path ends at the *earliest* extension match sitting at a token boundary — not the first extension in `.exe`/`.cmd`/`.bat`/`.com` list order, and not a substring inside another word — so a `.bat` wrapper followed by an `.exe` argument still quotes only the wrapper. | --- @@ -674,6 +688,12 @@ A coverage run over the non-UI packages reports these as uncovered. All are intentional; none is an oversight to be "fixed" with a test. - The real `Clock` — a fake is injected everywhere it is used. -- `storage.OpenStore`, `storage.ResolvePaths`, `storage.PeekKeepRunningInTray`, `app.Service.Start`, `app.Service.Open` — process entry points, exercised by running the app. -- The autostart and desktop-icon wrappers — OS integration, driven only on a real desktop. +- `storage.OpenStore`, `storage.ResolvePaths`, `storage.PeekKeepRunningInTray`, `app.Service.Start`, `app.Open` — process entry points, exercised by running the app. +- The autostart and desktop-icon wrappers (`app.Service.InstallDesktopIcon`, `AutostartStatus`, `ApplyAutostart`) — OS integration, driven only on a real desktop. - `app.Service.ShouldNotifyOnFailure` — a getter under the mutex. +- `app.Service.Config` and `app.Service.Paths` — read only from `src/ui`, which + this run excludes, so they are covered by the suite but not by this profile. + The same applies to `storage.Store.SaveJobs`: the engine writes through + `PrepareSaveJobs`, and the one-shot wrapper is what `OpenStore` uses. +- The five `isEvent` marker methods in `app/events.go` — empty bodies that exist + only to close the `Event` interface.