Compare commits

8 Commits

Author SHA1 Message Date
mix 2ef18e759c test: delete duplicate-coverage tests, fix TESTS.md drift, drop hand-rolled itoa
Items 1-3 of the 2026-08-04 test-suite review: TestCleanupLogsKeepsFilesWithinAgeLimit,
TestRunDueEmptyOverlapInheritsGlobal, and TestSameWindowsPathHandlesSpaces had
byte-identical coverage to an existing test and no assertion the survivor lacked.
storage.defaultJobs, the one accidental 0% coverage gap the review found, is now
covered and TESTS.md corrected to match. seed_test.go's itoa is replaced with
strconv.FormatInt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-04 22:30:27 +03:00
mix 77bd2db286 docs: recommend a model per item in the test review plan
The deciding factor is the slow feedback loop, not task size: the ui
package needs CGO and the MSYS2 toolchain, and a cold test run took 258s
during the review. Getting an edit right on the first pass is worth more
than generating it faster.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:13:04 +03:00
mix 1b6a3604cf docs: add the test-suite review action plan
Records the findings of a review of the whole test suite: three duplicate
tests confirmed by comparing coverage profiles, two inaccuracies in
TESTS.md, a hand-rolled itoa in seed_test.go, and four tests thin enough
to need a decision.

Also lists the pairs that share a coverage profile but assert different
properties, and the functions whose zero coverage is deliberate, so a
later pass does not re-report them. Temporary: delete once the items are
done or moved to ROADMAP.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:11:50 +03:00
mix cb37377346 Add top inset to the Disable auto row in the Jobs sidebar.
The row sat flush against the tab bar while the VBox gap below was already one theme padding; matching that on top balances the spacing.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 22:10:11 +03:00
mix b2402f4c72 Rename the Default theme option to System.
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 21:49:06 +03:00
mix 648325a690 Point the About repository link at GitHub.
The Settings About block now links to mixeme/gosentry instead of the private Gitea mirror.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:35:29 +03:00
mix 27927f3ab1 docs: expand README @every schedule syntax
Document supported Go duration units, combinations, cron alternatives for calendar intervals, the one-second tick floor, and cron descriptors.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:28:42 +03:00
mix 276539c383 Make the branded GoSentry theme the default.
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 19:18:48 +03:00
16 changed files with 382 additions and 148 deletions
+41 -8
View File
@@ -33,7 +33,7 @@ creating, grouping, pausing, running, and monitoring scheduled shell commands.
- Desktop notifications on job failure. - Desktop notifications on job failure.
- Windows tray icon: left-click to show the window, right-click for the menu. - Windows tray icon: left-click to show the window, right-click for the menu.
- Autostart on login (Windows shortcut; Linux XDG desktop entry). - Autostart on login (Windows shortcut; Linux XDG desktop entry).
- Detailed or compact job list, and a default or branded theme; both are remembered. - Detailed or compact job list, and a system or branded theme; both are remembered.
## Platforms ## Platforms
@@ -74,7 +74,7 @@ portable application: moving the program folder also moves its configuration.
"execution_mode": "parallel", "execution_mode": "parallel",
"overlap_policy": "skip", "overlap_policy": "skip",
"default_timeout_seconds": 0, "default_timeout_seconds": 0,
"theme": "default", "theme": "gosentry",
"job_list_view": "detailed" "job_list_view": "detailed"
} }
``` ```
@@ -83,7 +83,7 @@ That is the file GoSentry writes on first run. `default_timeout_seconds` is the
run timeout applied to jobs that do not set their own; `0` means no timeout, and run timeout applied to jobs that do not set their own; `0` means no timeout, and
it is written out even though it is zero, because a missing value and a it is written out even though it is zero, because a missing value and a
deliberate "no timeout" have to stay distinguishable in a hand-edited file. deliberate "no timeout" have to stay distinguishable in a hand-edited file.
`theme` is `default` or `gosentry` (the branded teal/amber look), and `theme` is `system` or `gosentry` (the branded teal/amber look), and
`job_list_view` is `detailed` or `compact` — both are remembered from the `job_list_view` is `detailed` or `compact` — both are remembered from the
choices made in the app. Keys left at their off value (`start_on_login`, choices made in the app. Keys left at their off value (`start_on_login`,
`paused`) are omitted until they are turned on. `paused`) are omitted until they are turned on.
@@ -126,22 +126,55 @@ include the run timestamp and job name:
## Schedules ## Schedules
Interval schedules using Go duration syntax: GoSentry accepts two schedule forms: fixed `@every` intervals and standard
5-field cron expressions.
### `@every` intervals
Write `@every` followed by a [Go duration](https://pkg.go.dev/time#ParseDuration)
— a positive number with a unit suffix. Units can be combined in one value:
```text ```text
@every 10s @every 10s every 10 seconds
@every 5m @every 5m every 5 minutes
@every 1h30m @every 1h every hour
@every 1h30m every hour and a half (same as @every 90m)
@every 2h45m10s hours, minutes, and seconds combined
``` ```
Standard 5-field cron expressions: Supported units:
| Unit | Meaning |
|------|---------|
| `ns` | nanoseconds |
| `us`, `µs` | microseconds |
| `ms` | milliseconds |
| `s` | seconds |
| `m` | minutes |
| `h` | hours |
`@every` does **not** support days, weeks, months, or years — those follow a
calendar, not a fixed interval. For “every day at 02:00”, “on the 1st of each
month”, or “once a year”, use a cron expression (below).
The scheduler checks due jobs once per second, so values shorter than `1s` are
accepted but will not fire faster than once a second.
### Cron expressions
Five fields: minute, hour, day-of-month, month, day-of-week.
```text ```text
*/5 * * * * every five minutes */5 * * * * every five minutes
0 2 * * * every day at 02:00 0 2 * * * every day at 02:00
30 9 * * 1-5 weekdays at 09:30 30 9 * * 1-5 weekdays at 09:30
0 0 1 * * first day of every month at midnight
0 0 1 1 * every year on 1 January at midnight
``` ```
Named descriptors are also accepted: `@hourly`, `@daily`, `@weekly`,
`@monthly`, `@yearly` (and `@annually`, `@midnight`).
## Using The App ## Using The App
1. Start GoSentry. 1. Start GoSentry.
+33
View File
@@ -38,6 +38,12 @@ dragged.**
**Settings:** **Settings:**
- **The branded GoSentry theme is now the default.** Fresh installs, the
**Defaults** button, and configs that omit `theme` all open in the teal/amber
look; users who prefer Fyne's built-in theme can still pick **System** in
Settings.
- The Fyne built-in theme option is labelled **System** (stored as `"system"`);
configs that still say `"default"` are read as System and rewritten on save.
- The **Save / Cancel / Restore defaults** row sits 4 px from the left edge, as - The **Save / Cancel / Restore defaults** row sits 4 px from the left edge, as
its layout always intended, rather than 8. its layout always intended, rather than 8.
- The caption column is as wide as the widest caption instead of a fixed width, - The caption column is as wide as the widest caption instead of a fixed width,
@@ -51,9 +57,16 @@ dragged.**
the rows above have to give but a dropdown — which paints its box out to the the rows above have to give but a dropdown — which paints its box out to the
row's edge — does not, so the gap collapsed to about a pixel. The Theme row row's edge — does not, so the gap collapsed to about a pixel. The Theme row
now keeps the same gap the checkbox rows have. now keeps the same gap the checkbox rows have.
- The **About** repository link points at GitHub (`mixeme/gosentry`) instead of
the private Gitea mirror.
**Documentation:** **Documentation:**
- The **README Schedules** section now documents `@every` in full: supported Go
duration units (`ns` through `h`), combined values such as `1h30m`, the link to
`time.ParseDuration`, the fact that days/months/years belong in cron rather
than `@every`, the one-second scheduler tick floor, cron examples for monthly
and yearly runs, and the `@hourly`/`@daily`/… descriptors.
- The **README** describes the application that exists. Its `gosentry.json` - The **README** describes the application that exists. Its `gosentry.json`
sample was three keys short of what the app writes on first run, which made sample was three keys short of what the app writes on first run, which made
the one file the user is invited to hand-edit the least accurate thing in the the one file the user is invited to hand-edit the least accurate thing in the
@@ -91,6 +104,24 @@ dragged.**
split in one pass during the next whole-project review, since six separate split in one pass during the next whole-project review, since six separate
passes would settle the same seam question six ways. passes would settle the same seam question six ways.
**Tests:**
- Three tests with byte-identical coverage to an existing test and no unique
assertion are gone: `TestCleanupLogsKeepsFilesWithinAgeLimit`,
`TestRunDueEmptyOverlapInheritsGlobal` (its one unique setup guard moved into
`TestRunDueQueueRerunsAfterFinish`), and `TestSameWindowsPathHandlesSpaces`
(its spaces case folded into `TestSameWindowsPathIgnoresCaseAndQuotes`'s
fixture).
- `storage.defaultJobs` — the one accidental 0%-coverage gap the review
found — is now exercised by
`TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun`, which also corrects
`docs/TESTS.md`: `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` never
touched jobs, so the "and a sample job" half of its old description was
wrong.
- `src/runner/seed_test.go`'s hand-rolled `itoa` — 18 lines of digit-by-digit
conversion in a file that already imports `strconv` — is replaced with
`strconv.FormatInt`.
## 0.15.0 - 2026-07-26 ## 0.15.0 - 2026-07-26
**Settings points at the jobs file itself, not the folder holding it.** **Settings points at the jobs file itself, not the folder holding it.**
@@ -153,6 +184,8 @@ button for the logs folder.**
**Jobs sidebar:** **Jobs sidebar:**
- The **Disable auto** row gained a top inset matching the gap below it, so it
no longer sits flush against the tab bar.
- The **Folder** caption moved onto the filter row itself, beside the select and - The **Folder** caption moved onto the filter row itself, beside the select and
the view toggle, instead of occupying its own line above it — the job list now the view toggle, instead of occupying its own line above it — the job list now
starts a full label higher. starts a full label higher.
+9 -9
View File
@@ -175,11 +175,10 @@ and scheduler edge cases using injected `runJob` and `primeDue`.
| `TestRunDueParallelStartsAllDueJobs` | Parallel mode: both due jobs enter the runner before either completes. | | `TestRunDueParallelStartsAllDueJobs` | Parallel mode: both due jobs enter the runner before either completes. |
| `TestRunDueSequentialSerializes` | Sequential mode: job 2 waits until job 1 finishes. | | `TestRunDueSequentialSerializes` | Sequential mode: job 2 waits until job 1 finishes. |
| `TestRunDueSkipDropsOverlap` | Global skip: no second concurrent run, `PendingRuns` stays 0. | | `TestRunDueSkipDropsOverlap` | Global skip: no second concurrent run, `PendingRuns` stays 0. |
| `TestRunDueQueueRerunsAfterFinish` | Queue: one deferred run after an in-flight finish. | | `TestRunDueQueueRerunsAfterFinish` | Queue: one deferred run after an in-flight finish; also covers an empty per-job policy inheriting the global default. |
| `TestRunDueQueueDrainsMultipleOverlaps` | Queue: multiple missed ticks drain as separate runs. | | `TestRunDueQueueDrainsMultipleOverlaps` | Queue: multiple missed ticks drain as separate runs. |
| `TestRunDuePerJobQueueOverridesGlobalSkip` | Per-job `queue` beats global `skip`. | | `TestRunDuePerJobQueueOverridesGlobalSkip` | Per-job `queue` beats global `skip`. |
| `TestRunDuePerJobSkipOverridesGlobalQueue` | Per-job `skip` beats global `queue`. | | `TestRunDuePerJobSkipOverridesGlobalQueue` | Per-job `skip` beats global `queue`. |
| `TestRunDueEmptyOverlapInheritsGlobal` | Empty per-job policy inherits the global default. |
| `TestRunNowSequentialGuard` | Manual run refused while another job runs in sequential mode. | | `TestRunNowSequentialGuard` | Manual run refused while another job runs in sequential mode. |
| `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. | | `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. |
| `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. | | `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. |
@@ -234,9 +233,11 @@ Tests JSON round-tripping, default generation, and backward compatibility.
| `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. | | `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. |
| `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. | | `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. |
| `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. | | `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. |
| `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults and a sample job. | | `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. | | `TestLoadOrCreateConfigKeepsZeroTimeoutOnReload` | Verifies that `default_timeout_seconds: 0` survives a reload rather than being normalized away — 0 is a value, not a missing field. |
| `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. | | `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. | | `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. | | `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. | | `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. |
@@ -354,8 +355,7 @@ Tests log-file cleanup by age and by count.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestCleanupLogsMissingDirReturnsNil` | Verifies that cleanup returns nil (not an error) when the logs directory does not exist. | | `TestCleanupLogsMissingDirReturnsNil` | Verifies that cleanup returns nil (not an error) when the logs directory does not exist. |
| `TestCleanupLogsRemovesFilesPastMaxAge` | Verifies that `.log` files older than `MaxLogAgeDays` are deleted. | | `TestCleanupLogsRemovesFilesPastMaxAge` | Verifies that `.log` files older than `MaxLogAgeDays` are deleted and files within the limit are retained. |
| `TestCleanupLogsKeepsFilesWithinAgeLimit` | Verifies that `.log` files within the age limit are retained. |
| `TestCleanupLogsByCountDeletesOldest` | Verifies that when file count exceeds `MaxLogFiles`, the oldest files are removed first. | | `TestCleanupLogsByCountDeletesOldest` | Verifies that when file count exceeds `MaxLogFiles`, the oldest files are removed first. |
| `TestCleanupLogsNonLogFilesNotDeleted` | Verifies that non-`.log` files in the logs directory are never deleted by cleanup. | | `TestCleanupLogsNonLogFilesNotDeleted` | Verifies that non-`.log` files in the logs directory are never deleted by cleanup. |
| `TestCleanupLogsSubdirsNotDeleted` | Verifies that subdirectories inside the logs directory are not deleted by cleanup. | | `TestCleanupLogsSubdirsNotDeleted` | Verifies that subdirectories inside the logs directory are not deleted by cleanup. |
@@ -372,8 +372,7 @@ Tests Windows autostart via shortcuts in the Startup folder.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestSameWindowsPathIgnoresCaseAndQuotes` | Verifies that Windows path comparison is case-insensitive and handles quote marks correctly. | | `TestSameWindowsPathIgnoresCaseAndQuotes` | Verifies that Windows path comparison is case-insensitive, handles quote marks, and matches paths containing spaces. |
| `TestSameWindowsPathHandlesSpaces` | Verifies that Windows path comparison matches paths with and without surrounding quotes. |
| `TestSameWindowsPathStripsExtendedLengthPrefix` | Verifies that `\\?\`-prefixed paths are compared correctly after stripping the prefix. | | `TestSameWindowsPathStripsExtendedLengthPrefix` | Verifies that `\\?\`-prefixed paths are compared correctly after stripping the prefix. |
| `TestSameWindowsPathMatchesShortNameViaFilesystem` | Verifies that 8.3 short names are resolved to long names for comparison. | | `TestSameWindowsPathMatchesShortNameViaFilesystem` | Verifies that 8.3 short names are resolved to long names for comparison. |
| `TestStartupShortcutPathUsesUserStartupFolder` | Verifies that the shortcut path resolves into `%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup`. | | `TestStartupShortcutPathUsesUserStartupFolder` | Verifies that the shortcut path resolves into `%APPDATA%\Microsoft\Windows\Start Menu\Programs\Startup`. |
@@ -500,6 +499,7 @@ Tests the theme-derived sizing helpers in `layout.go`.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestRowOverlapMatchesInnerPadding` | Pins `rowOverlap` to `-theme.InnerPadding()` under two themes, the property that lets it follow a theme instead of drifting from a hand-tuned literal. | | `TestRowOverlapMatchesInnerPadding` | Pins `rowOverlap` to `-theme.InnerPadding()` under two themes, the property that lets it follow a theme instead of drifting from a hand-tuned literal. |
| `TestCancelRowOverlapAddsBackOneInnerPadding` | Verifies that `cancelRowOverlap` adds back exactly one inner padding on the top edge only, leaving width and the row below unaffected. |
| `TestCaptionColumnWidth` | Covers no captions, one, and several of varying length, at two text sizes. | | `TestCaptionColumnWidth` | Covers no captions, one, and several of varying length, at two text sizes. |
--- ---
@@ -514,8 +514,8 @@ Tests the branded theme and the stored theme choice.
|------|---------| |------|---------|
| `TestGoSentryThemeBrandColors` | Verifies the brand colors land on the semantically correct `ColorName`s in both the light and dark variants. | | `TestGoSentryThemeBrandColors` | Verifies the brand colors land on the semantically correct `ColorName`s in both the light and dark variants. |
| `TestGoSentryThemeDelegatesUnbrandedColors` | Verifies unbranded color names fall through to the base theme rather than rendering transparent. | | `TestGoSentryThemeDelegatesUnbrandedColors` | Verifies unbranded color names fall through to the base theme rather than rendering transparent. |
| `TestThemeForChoice` | Verifies the GoSentry choice yields the branded primary and every other value — including the empty legacy one — yields the default theme. | | `TestThemeForChoice` | Verifies the GoSentry choice and the empty legacy value yield the branded primary; only the explicit system choice yields Fyne's built-in theme. |
| `TestThemeLabelRoundTrip` | Verifies the dropdown labels round-trip and that the empty value maps to the Default label rather than a blank option. | | `TestThemeLabelRoundTrip` | Verifies the dropdown labels round-trip and that the empty value maps to the GoSentry label rather than a blank option. |
--- ---
+178
View File
@@ -0,0 +1,178 @@
# Test-suite review — action plan
Working document for the findings of the 2026-08-04 review of the test suite.
It is not part of the permanent doc set: delete it once every item below is
either done or moved to [ROADMAP.md](ROADMAP.md).
The rules the findings were judged against live in [STANDARDS.md](STANDARDS.md);
the suite itself is described in [TESTS.md](TESTS.md).
## Baseline the review started from
- 172 tests, 4693 lines of test code.
- 84.4% statement coverage across `domain`, `storage`, `runner`, `scheduler`,
and `app` measured together with `-coverpkg` (per-package figures understate
it, because e.g. `domain.NewRuntime` is exercised from the `app` tests).
- [TESTS.md](TESTS.md) documents 171 of the 172 tests.
Overall finding: the suite is **not** padded. Every test but one carries a real
assertion, and most record the property they pin. The items below are the
exceptions.
Method note: redundancy was not judged by reading. Each suspected pair was run
in isolation with `-coverprofile` and the profiles compared. "Identical
coverage" below means the two profiles were byte-identical after sorting.
Identical coverage alone is *not* grounds for deletion — several kept tests hit
the same statements while asserting genuinely different properties. Deletion
requires identical coverage **and** assertions that are a subset.
## 1. Delete the measured duplicates
Each of these has a byte-identical coverage profile with an existing test whose
assertions are a superset. Roughly 30 lines total.
- [x] `TestCleanupLogsKeepsFilesWithinAgeLimit`
([cleanup_test.go:53](../src/runner/cleanup_test.go)) — delete.
`TestCleanupLogsRemovesFilesPastMaxAge` already asserts that the file
inside the age limit survives.
- [x] `TestRunDueEmptyOverlapInheritsGlobal`
([run_test.go:457](../src/app/run_test.go)) — delete. It builds the same
service as `TestRunDueQueueRerunsAfterFinish` (parallel mode, global
`queue`, a job with an empty `OverlapPolicy`) and asserts strictly less.
Before deleting, move its one unique line — the setup guard
`svc.jobs[0].OverlapPolicy != ""` — into `TestRunDueQueueRerunsAfterFinish`,
so that test still states out loud that it is exercising the inherited
policy rather than an explicit one.
- [x] `TestSameWindowsPathHandlesSpaces`
([autostart_windows_test.go:20](../src/platform/autostart/autostart_windows_test.go))
— delete. It is the same case as `TestSameWindowsPathIgnoresCaseAndQuotes`
(quoted path, mixed case); `sameWindowsPath` does not split on spaces, so
the space in the fixture reaches no new code. If the spaces case is worth
naming, fold the path into the surviving test's fixture instead.
After the deletions, re-run the affected packages and confirm coverage is
unchanged:
```bash
go test -coverpkg=./src/domain,./src/storage,./src/runner,./src/scheduler,./src/app ./src/domain ./src/storage ./src/runner ./src/scheduler ./src/app
```
## 2. Fix the documentation drift
- [x] [TESTS.md](TESTS.md) claims `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun`
verifies that a missing config file is created "with sane defaults **and a
sample job**". The test never touches jobs, and `storage.defaultJobs` sits
at 0% coverage. Decide which half is wrong: either drop the claim from the
table, or add the assertion that the seeded `jobs.json` contains the
sample jobs. Adding the assertion is the better outcome — `defaultJobs` is
the only accidental coverage gap the review found.
- [x] [TESTS.md](TESTS.md) does not list
`TestCancelRowOverlapAddsBackOneInnerPadding`
([layout_test.go:35](../src/ui/layout_test.go)). Add it to the
`src/ui/layout_test.go` table.
## 3. Replace the hand-rolled helper in test code
- [x] [seed_test.go:34](../src/runner/seed_test.go) defines `itoa`: 18 lines of
digit-by-digit conversion with a fresh allocation per digit, in a file
that already imports `strconv`. Replace the calls with
`strconv.FormatInt` and delete the helper. Untested logic inside a test
file is exactly what produces a test result nobody can trust.
## 4. Thin tests — decide, then act
None of these is wrong; each is close enough to worthless that it should be
either justified or removed. Grouped because they want one decision, not four.
- [ ] `TestEmitWithNoObserversIsNoop`
([events_test.go:36](../src/app/events_test.go)) — the only test in the
suite with no assertion at all. Ranging over a nil slice cannot panic in
Go, so it pins nothing. Delete.
- [ ] `TestStoreReturnsWiredStore`
([service_test.go:51](../src/app/service_test.go)) — asserts that a
one-line getter returns its own field. Delete.
- [ ] `TestMainViewBuilds`
([mainwindow_test.go:72](../src/ui/mainwindow_test.go)) — a smoke test;
`TestMainViewFitsTheDefaultWindowSize` builds the same view. Its only
unique coverage is `w.SetContent(content)` and `recordStartup(0, true)`.
Either fold those two calls into the sizing test and delete this one, or
keep it and say in its comment that `recordStartup` is what it is for.
- [ ] `TestFilteredJobIndexesAll` / `ByNamedFolder` / `NoFolder` / `EmptySlice`
([jobs_view_test.go:59-101](../src/ui/jobs_view_test.go)) — four tests
over one small pure function. Collapse into one table-driven test in the
style of `TestFilterValue` directly above them; the `EmptySlice` case
becomes one row rather than a function.
## 5. Runtime cost of the runner tests (optional)
`TestRunJobLogFileAllHeaders`, `TestRunJobRecordFields`, and
`TestRunJobWritesLogFile` ([runner_test.go](../src/runner/runner_test.go)) have
identical coverage profiles but assert three genuinely different things — log
headers, `RunRecord` field values, and the log file's name and directory. They
are **not** duplicates and should not be deleted on that basis.
The cost is that each spawns a real subprocess; the `runner` package takes 5.3 s.
If suite wall time becomes a concern, merge them into one `RunJob` call with
three assertion blocks. Until then, leave them alone.
## Explicitly not changing
Recorded here so a later pass does not re-report them:
- `TestSetGlobalPausePersistsToConfigFile` has the same coverage as
`TestSetGlobalPauseUpdatesRuntimesAndEmits` but asserts a different property —
that the flag reaches `gosentry.json`, which is what makes the pause survive a
restart.
- `TestRunDueQueueDrainsMultipleOverlaps` has the same coverage as
`TestRunDueQueueRerunsAfterFinish`, but drains three queued occurrences rather
than one. It is the test that would catch a drain loop that fires once.
- `TestCreateStartupShortcutHandlesCyrillicPath` and `...HandlesSpaces` cover the
same statements, but non-ASCII paths and paths with spaces are different
real-world failure modes for the WScript.Shell COM call.
- The 0% functions in the non-UI packages are deliberate: the real `Clock` (a
fake is injected everywhere), `OpenStore` / `ResolvePaths` / `Service.Start` /
`Service.Open` and the autostart and desktop-icon wrappers (process entry
points and OS integration), and `ShouldNotifyOnFailure` (a getter under the
mutex). `storage.defaultJobs` is the exception — see item 2.
## Suggested order
1. Item 2 (docs) — smallest, and the `defaultJobs` assertion is the only one
that adds coverage.
2. Item 3 (`itoa`) — independent of everything else.
3. Item 1 (deletions) — one commit, with the coverage re-run as evidence.
4. Item 4 (thin tests) — needs a judgment call per test.
5. Item 5 — only if suite wall time becomes a problem.
Items 1 and 4 change the test inventory, so [TESTS.md](TESTS.md) has to be
updated in the same commit. No [CHANGELOG.md](CHANGELOG.md) entry is needed:
none of this changes shipped behavior.
## Which model to use
For running these items in Claude Code. The deciding factor here is not task
size — it is that **the feedback loop is slow**: the `ui` package needs the
MSYS2 UCRT64 toolchain with CGO on, and a cold `go test ./src/ui/...` took
**258 s** during the review. A model that gets an edit right on the first pass
is worth more than a faster one that needs a second build to find out.
| Item | Model | Why |
|---|---|---|
| 3 — `itoa``strconv.FormatInt` | **Haiku 4.5** (`claude-haiku-4-5`) | A mechanical substitution in one file, in the `runner` package, which needs no CGO and runs in ~5 s. Nothing to weigh. |
| 2 — docs, and the `defaultJobs` assertion | **Sonnet 5** (`claude-sonnet-5`) | Two doc edits plus one new assertion in `storage`. Reading `loadOrCreateJobs` to write the assertion is real work, but the answer is not in doubt. No CGO. |
| 1 — the three deletions | **Sonnet 5** | Deleting is easy; the judgment is narrow and already made in this document (which line to carry over from `TestRunDueEmptyOverlapInheritsGlobal`, and that identical coverage must be re-verified afterwards). One of the three is in `platform/autostart`, which is Windows-gated but CGO-free. |
| 4 — the four thin tests | **Opus 5** (`claude-opus-5`) | This is the only item that is genuinely a judgment call rather than an execution task: whether each test should exist at all, and — for `TestMainViewBuilds` — whether to fold two calls into the sizing test or keep it with a better comment. Two of the four are in `ui`, so a wrong call costs a 4-minute rebuild to discover. |
| 5 — merging the runner tests | **Opus 5**, if attempted | It requires holding three distinct sets of assertions and confirming none is silently dropped in the merge. It is also the item most likely to be *not worth doing* — a model that will say so is the point. |
Two notes on this table:
- **Sonnet 5 is the reasonable single choice** if you would rather not switch
models per item. It is near-Opus on coding and agentic work, and only item 4
really rewards the step up. The introductory pricing through **2026-08-31**
($2/$10 per MTok vs $3/$15) makes it cheaper than usual relative to Opus 5's
$5/$25.
- **Fast mode is available on Opus 5** (toggle with `/fast`). It is the same
model with higher output throughput, not a downgrade — but it bills at
$10/$50, so it only pays for itself when you are waiting on the output. Given
that the actual wait here is the Fyne build rather than token generation, it
is unlikely to help on this plan.
+4 -4
View File
@@ -481,10 +481,10 @@ func validateConfig(config domain.Config) error {
if config.DefaultTimeoutSeconds < 0 { if config.DefaultTimeoutSeconds < 0 {
return errors.New("default timeout must not be negative (0 means no timeout)") return errors.New("default timeout must not be negative (0 means no timeout)")
} }
// Empty Theme is accepted and normalized to the default on load, so older // Empty Theme is accepted and normalized to the branded theme on load, so
// configs (and hand-built ones) stay valid without an explicit theme. // older configs (and hand-built ones) stay valid without an explicit theme.
if config.Theme != "" && config.Theme != domain.ThemeDefault && config.Theme != domain.ThemeGoSentry { if config.Theme != "" && config.Theme != domain.ThemeSystem && config.Theme != domain.ThemeGoSentry {
return errors.New("theme must be 'default' or 'gosentry'") return errors.New("theme must be 'system' or 'gosentry'")
} }
return nil return nil
} }
+4 -45
View File
@@ -255,6 +255,10 @@ func TestRunDueQueueRerunsAfterFinish(t *testing.T) {
svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{ svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{
{ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true}, {ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true},
}) })
// Empty per-job OverlapPolicy inherits the global queue policy.
if svc.jobs[0].OverlapPolicy != "" {
t.Fatalf("test setup: job OverlapPolicy = %q, want empty (inherit)", svc.jobs[0].OverlapPolicy)
}
entered := make(chan int, 2) entered := make(chan int, 2)
release := make(chan struct{}) release := make(chan struct{})
@@ -451,51 +455,6 @@ func TestRunDuePerJobSkipOverridesGlobalQueue(t *testing.T) {
} }
} }
// TestRunDueEmptyOverlapInheritsGlobal verifies that a job with no own policy
// inherits the global default: with global "queue" and an empty Job.OverlapPolicy
// the job queues a re-run.
func TestRunDueEmptyOverlapInheritsGlobal(t *testing.T) {
svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{
{ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true},
})
if svc.jobs[0].OverlapPolicy != "" {
t.Fatalf("test setup: job OverlapPolicy = %q, want empty (inherit)", svc.jobs[0].OverlapPolicy)
}
entered := make(chan int, 2)
release := make(chan struct{})
svc.runJob = func(_ context.Context, job *domain.Job, _ string, _ string, _ time.Duration) (domain.RunRecord, error) {
entered <- job.ID
<-release
return domain.RunRecord{Time: "t", JobID: job.ID, JobName: job.Name, State: "Success"}, nil
}
done := completions(svc)
primeDue(t, svc, 1)
svc.RunDue(time.Now())
if id := <-entered; id != 1 {
t.Fatalf("started job = %d, want 1", id)
}
primeDue(t, svc, 1)
svc.RunDue(time.Now())
expectNoEntry(t, entered)
svc.mu.Lock()
pending := svc.runtimes[1].PendingRuns
svc.mu.Unlock()
if pending != 1 {
t.Fatalf("empty per-job policy must inherit global queue, PendingRuns = %d", pending)
}
close(release)
waitRecord(t, done)
if id := <-entered; id != 1 {
t.Fatalf("inherited-queue re-run job = %d, want 1", id)
}
waitRecord(t, done)
}
// TestRunNowSequentialGuard verifies the sequential-mode guard in RunNow: a manual // TestRunNowSequentialGuard verifies the sequential-mode guard in RunNow: a manual
// run is refused while another job is running, and allowed once nothing is. // run is refused while another job is running, and allowed once nothing is.
func TestRunNowSequentialGuard(t *testing.T) { func TestRunNowSequentialGuard(t *testing.T) {
+5 -5
View File
@@ -21,8 +21,8 @@ const (
type Theme string type Theme string
const ( const (
// ThemeDefault keeps Fyne's built-in theme — the original look. // ThemeSystem keeps Fyne's built-in theme, following the platform look.
ThemeDefault Theme = "default" ThemeSystem Theme = "system"
// ThemeGoSentry applies the branded teal/amber theme derived from the logo // ThemeGoSentry applies the branded teal/amber theme derived from the logo
// and app icon. // and app icon.
ThemeGoSentry Theme = "gosentry" ThemeGoSentry Theme = "gosentry"
@@ -87,8 +87,8 @@ type Config struct {
// omitempty would hide a deliberate choice from the hand-editable config. // omitempty would hide a deliberate choice from the hand-editable config.
DefaultTimeoutSeconds int `json:"default_timeout_seconds"` DefaultTimeoutSeconds int `json:"default_timeout_seconds"`
Paused bool `json:"paused,omitempty"` Paused bool `json:"paused,omitempty"`
// Theme selects the visual appearance. Empty is treated as ThemeDefault so // Theme selects the visual appearance. Empty is treated as ThemeGoSentry so
// configs written before this field existed keep the original look. // configs written before this field existed pick up the branded look.
Theme Theme `json:"theme,omitempty"` Theme Theme `json:"theme,omitempty"`
// JobListView selects the Jobs list density. Empty is treated as // JobListView selects the Jobs list density. Empty is treated as
// JobListViewDetailed so configs written before this field existed keep the // JobListViewDetailed so configs written before this field existed keep the
@@ -110,7 +110,7 @@ func DefaultConfig() Config {
NotifyOnFailure: true, NotifyOnFailure: true,
ExecutionMode: ExecutionModeParallel, ExecutionMode: ExecutionModeParallel,
OverlapPolicy: OverlapPolicySkip, OverlapPolicy: OverlapPolicySkip,
Theme: ThemeDefault, Theme: ThemeGoSentry,
JobListView: JobListViewDetailed, JobListView: JobListViewDetailed,
DefaultTimeoutSeconds: 0, DefaultTimeoutSeconds: 0,
} }
@@ -12,14 +12,8 @@ import (
) )
func TestSameWindowsPathIgnoresCaseAndQuotes(t *testing.T) { func TestSameWindowsPathIgnoresCaseAndQuotes(t *testing.T) {
if !sameWindowsPath(`"D:\Apps\GoSentry\gosentry.exe"`, `d:\apps\gosentry\gosentry.exe`) {
t.Fatal("expected paths to match")
}
}
func TestSameWindowsPathHandlesSpaces(t *testing.T) {
if !sameWindowsPath(`"D:\Local Git\GoSentry\gosentry.exe"`, `d:\local git\gosentry\gosentry.exe`) { if !sameWindowsPath(`"D:\Local Git\GoSentry\gosentry.exe"`, `d:\local git\gosentry\gosentry.exe`) {
t.Fatal("expected paths with spaces to match") t.Fatal("expected paths to match")
} }
} }
-16
View File
@@ -50,22 +50,6 @@ func TestCleanupLogsRemovesFilesPastMaxAge(t *testing.T) {
} }
} }
func TestCleanupLogsKeepsFilesWithinAgeLimit(t *testing.T) {
dir := t.TempDir()
for i := 1; i <= 3; i++ {
path := writeLogFile(t, dir, fmt.Sprintf("job_%d.log", i))
setModTime(t, path, time.Duration(i)*24*time.Hour)
}
if err := CleanupLogs(dir, 100, 30); err != nil {
t.Fatal(err)
}
entries, _ := os.ReadDir(dir)
if len(entries) != 3 {
t.Errorf("expected 3 files kept within age limit, got %d", len(entries))
}
}
// TestCleanupLogsByCountDeletesOldest verifies the count-based policy: when more // TestCleanupLogsByCountDeletesOldest verifies the count-based policy: when more
// than maxFiles log files exist the oldest (by modification time) are removed. // than maxFiles log files exist the oldest (by modification time) are removed.
// maxAgeDays=0 disables age-based cleanup so the test exercises count only. // maxAgeDays=0 disables age-based cleanup so the test exercises count only.
+1 -20
View File
@@ -22,7 +22,7 @@ func writeTestLog(t *testing.T, dir, filename, state string, durationMS int64, j
content.WriteString("\n") content.WriteString("\n")
} }
if durationMS >= 0 { if durationMS >= 0 {
content.WriteString("state: " + state + "\nduration: " + itoa(durationMS) + "\n\n") content.WriteString("state: " + state + "\nduration: " + strconv.FormatInt(durationMS, 10) + "\n\n")
} else { } else {
content.WriteString("state: " + state + "\n\n") content.WriteString("state: " + state + "\n\n")
} }
@@ -31,25 +31,6 @@ func writeTestLog(t *testing.T, dir, filename, state string, durationMS int64, j
} }
} }
func itoa(n int64) string {
if n == 0 {
return "0"
}
neg := n < 0
if neg {
n = -n
}
buf := make([]byte, 0, 20)
for n > 0 {
buf = append([]byte{byte('0' + n%10)}, buf...)
n /= 10
}
if neg {
buf = append([]byte{'-'}, buf...)
}
return string(buf)
}
func TestSeedStatsBasic(t *testing.T) { func TestSeedStatsBasic(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
job := domain.Job{ID: 1, Name: "Build"} job := domain.Job{ID: 1, Name: "Build"}
+4 -1
View File
@@ -118,7 +118,10 @@ func loadOrCreateConfig(paths Paths) (domain.Config, error) {
// the setting impossible to persist. Negative values are rejected by // the setting impossible to persist. Negative values are rejected by
// app.validateConfig before they can be saved. // app.validateConfig before they can be saved.
if config.Theme == "" { if config.Theme == "" {
config.Theme = domain.ThemeDefault config.Theme = domain.ThemeGoSentry
}
if config.Theme == "default" {
config.Theme = domain.ThemeSystem
} }
return config, nil return config, nil
} }
+65 -2
View File
@@ -171,8 +171,8 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
if got.DefaultTimeoutSeconds != 0 { if got.DefaultTimeoutSeconds != 0 {
t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds) t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds)
} }
if got.Theme != domain.ThemeDefault { if got.Theme != domain.ThemeGoSentry {
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeDefault) t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeGoSentry)
} }
if got.JobListView != domain.JobListViewDetailed { if got.JobListView != domain.JobListViewDetailed {
t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed) t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed)
@@ -183,6 +183,69 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
} }
} }
// TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing
// jobs.json is created with the sample jobs from defaultJobs, so a new user
// sees scheduled and manual execution without inventing a command.
func TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "jobs.json")
got, err := loadOrCreateJobs(path)
if err != nil {
t.Fatal(err)
}
want := defaultJobs()
if len(got) != len(want) {
t.Fatalf("got %d jobs, want %d", len(got), len(want))
}
for i := range want {
if got[i].Name != want[i].Name || got[i].Schedule != want[i].Schedule || got[i].Command != want[i].Command || got[i].Enabled != want[i].Enabled {
t.Errorf("job %d = %+v, want %+v", i, got[i], want[i])
}
}
// The function must have written the seeded jobs to jobs.json.
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("jobs.json should have been created: %v", err)
}
var file domain.JobsFile
if err := json.Unmarshal(data, &file); err != nil {
t.Fatal(err)
}
if len(file.Jobs) != len(want) {
t.Errorf("jobs.json has %d jobs, want %d", len(file.Jobs), len(want))
}
}
// TestLoadOrCreateConfigMigratesLegacyThemeDefault covers a gosentry.json that
// still stores the retired "default" theme value: load normalizes it to system.
func TestLoadOrCreateConfigMigratesLegacyThemeDefault(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
legacy := map[string]any{
"jobs_file": "jobs.json",
"logs_dir": "logs",
"max_log_files": 100,
"max_log_age_days": 30,
"theme": "default",
}
if err := writeJSON(paths.ConfigPath, legacy); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.Theme != domain.ThemeSystem {
t.Errorf("migrated Theme = %q, want %q", got.Theme, domain.ThemeSystem)
}
}
// TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout" // TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout"
// setting against being normalized away when an existing gosentry.json is read // setting against being normalized away when an existing gosentry.json is read
// back. Loading must not treat 0 as a missing value. // back. Loading must not treat 0 as a missing value.
+7 -1
View File
@@ -336,7 +336,13 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) {
}) })
toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer()) toolbar := container.NewHBox(addButton, editButton, runButton, pauseButton, deleteButton, layout.NewSpacer())
globalControls := container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer()) // The row sits directly under the tab bar with no AppTabs inset, while the
// default VBox gap below it is one theme padding — add the same on top so
// the button is not flush against the tabs.
globalControls := container.New(
layout.NewCustomPaddedLayout(theme.Padding(), 0, 0, 0),
container.NewHBox(stopAllButton, schedulerState, layout.NewSpacer()),
)
// The whole filter is one row: caption on the left, view toggle on the right, // The whole filter is one row: caption on the left, view toggle on the right,
// select filling what is left. The border layout gives both edges their // select filling what is left. The border layout gives both edges their
// MinSize, so the header is a line shorter than a stacked caption would make it. // MinSize, so the header is a line shorter than a stacked caption would make it.
+8 -8
View File
@@ -12,7 +12,7 @@ import (
"fyne.io/fyne/v2/widget" "fyne.io/fyne/v2/widget"
) )
const projectRepositoryURL = "https://gitea.mixdep.ru/mix/gosentry" const projectRepositoryURL = "https://github.com/mixeme/gosentry"
// settingsCaptions lists every settingsRow caption in the tab, in no // settingsCaptions lists every settingsRow caption in the tab, in no
// particular order. settingsView measures this once with captionColumnWidth // particular order. settingsView measures this once with captionColumnWidth
@@ -61,7 +61,7 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil) notifications := widget.NewCheck("Show desktop notifications for failed jobs", nil)
notifications.SetChecked(store.Config.NotifyOnFailure) notifications.SetChecked(store.Config.NotifyOnFailure)
notifications.OnChanged = func(bool) { updateSaveState() } notifications.OnChanged = func(bool) { updateSaveState() }
themeSelect := widget.NewSelect([]string{themeLabelDefault, themeLabelGoSentry}, nil) themeSelect := widget.NewSelect([]string{themeLabelSystem, themeLabelGoSentry}, nil)
themeSelect.SetSelected(themeLabel(store.Config.Theme)) themeSelect.SetSelected(themeLabel(store.Config.Theme))
// Preview the theme the moment it is picked so the choice is visible before // Preview the theme the moment it is picked so the choice is visible before
// saving; Save persists it. Reverting the selection reverts the preview, and // saving; Save persists it. Reverting the selection reverts the preview, and
@@ -256,22 +256,22 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject {
// Theme dropdown labels. These are the human-facing captions; themeLabel and // Theme dropdown labels. These are the human-facing captions; themeLabel and
// themeFromLabel translate between them and the stored domain.Theme values so the // themeFromLabel translate between them and the stored domain.Theme values so the
// select never leaks the on-disk "default"/"gosentry" strings to the user. // select never leaks the on-disk "system"/"gosentry" strings to the user.
const ( const (
themeLabelDefault = "Default" themeLabelSystem = "System"
themeLabelGoSentry = "GoSentry" themeLabelGoSentry = "GoSentry"
) )
func themeLabel(choice domain.Theme) string { func themeLabel(choice domain.Theme) string {
if choice == domain.ThemeGoSentry { if choice == domain.ThemeSystem {
return themeLabelGoSentry return themeLabelSystem
} }
return themeLabelDefault return themeLabelGoSentry
} }
func themeFromLabel(label string) domain.Theme { func themeFromLabel(label string) domain.Theme {
if label == themeLabelGoSentry { if label == themeLabelGoSentry {
return domain.ThemeGoSentry return domain.ThemeGoSentry
} }
return domain.ThemeDefault return domain.ThemeSystem
} }
+6 -6
View File
@@ -103,14 +103,14 @@ func (t gosentryTheme) Font(style fyne.TextStyle) fyne.Resource { return t.base.
func (t gosentryTheme) Icon(name fyne.ThemeIconName) fyne.Resource { return t.base.Icon(name) } func (t gosentryTheme) Icon(name fyne.ThemeIconName) fyne.Resource { return t.base.Icon(name) }
func (t gosentryTheme) Size(name fyne.ThemeSizeName) float32 { return t.base.Size(name) } func (t gosentryTheme) Size(name fyne.ThemeSizeName) float32 { return t.base.Size(name) }
// themeFor maps a stored Theme choice to a concrete fyne.Theme. Anything other // themeFor maps a stored Theme choice to a concrete fyne.Theme. Only the
// than the explicit GoSentry choice (including the empty/legacy value) keeps // explicit system choice keeps Fyne's built-in theme; everything else
// Fyne's built-in theme. // (including the empty/legacy value) uses the branded GoSentry theme.
func themeFor(choice domain.Theme) fyne.Theme { func themeFor(choice domain.Theme) fyne.Theme {
if choice == domain.ThemeGoSentry { if choice == domain.ThemeSystem {
return newGoSentryTheme() return theme.DefaultTheme()
} }
return theme.DefaultTheme() return newGoSentryTheme()
} }
// applyTheme installs the theme for the given choice on the running app. Fyne // applyTheme installs the theme for the given choice on the running app. Fyne
+16 -16
View File
@@ -54,32 +54,32 @@ func TestGoSentryThemeDelegatesUnbrandedColors(t *testing.T) {
} }
} }
// themeFor maps the stored choice to the right theme: the GoSentry choice yields // themeFor maps the stored choice to the right theme: the GoSentry choice and the
// the branded teal primary; every other value (including the empty legacy value) // empty legacy value yield the branded teal primary; only the explicit system
// yields the default theme, whose primary is not the brand teal. // choice yields Fyne's built-in theme.
func TestThemeForChoice(t *testing.T) { func TestThemeForChoice(t *testing.T) {
gosentry := themeFor(domain.ThemeGoSentry) for _, choice := range []domain.Theme{domain.ThemeGoSentry, ""} {
if got := gosentry.Color(theme.ColorNamePrimary, theme.VariantLight); got != brandTeal { branded := themeFor(choice)
t.Errorf("themeFor(gosentry) primary = %v, want brand teal %v", got, brandTeal) if got := branded.Color(theme.ColorNamePrimary, theme.VariantLight); got != brandTeal {
} t.Errorf("themeFor(%q) primary = %v, want brand teal %v", choice, got, brandTeal)
for _, choice := range []domain.Theme{domain.ThemeDefault, ""} {
def := themeFor(choice)
if got := def.Color(theme.ColorNamePrimary, theme.VariantLight); got == brandTeal {
t.Errorf("themeFor(%q) should not use the brand teal primary", choice)
} }
} }
sys := themeFor(domain.ThemeSystem)
if got := sys.Color(theme.ColorNamePrimary, theme.VariantLight); got == brandTeal {
t.Errorf("themeFor(system) should not use the brand teal primary")
}
} }
// The dropdown label helpers must round-trip, and the empty/legacy value must map // The dropdown label helpers must round-trip, and the empty/legacy value must map
// to the Default label so the select never shows a blank option. // to the GoSentry label so the select never shows a blank option.
func TestThemeLabelRoundTrip(t *testing.T) { func TestThemeLabelRoundTrip(t *testing.T) {
if got := themeFromLabel(themeLabel(domain.ThemeGoSentry)); got != domain.ThemeGoSentry { if got := themeFromLabel(themeLabel(domain.ThemeGoSentry)); got != domain.ThemeGoSentry {
t.Errorf("round-trip gosentry = %q", got) t.Errorf("round-trip gosentry = %q", got)
} }
if got := themeFromLabel(themeLabel(domain.ThemeDefault)); got != domain.ThemeDefault { if got := themeFromLabel(themeLabel(domain.ThemeSystem)); got != domain.ThemeSystem {
t.Errorf("round-trip default = %q", got) t.Errorf("round-trip system = %q", got)
} }
if got := themeLabel(""); got != themeLabelDefault { if got := themeLabel(""); got != themeLabelGoSentry {
t.Errorf("empty theme label = %q, want %q", got, themeLabelDefault) t.Errorf("empty theme label = %q, want %q", got, themeLabelGoSentry)
} }
} }