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.