Compare commits

..

2 Commits

Author SHA1 Message Date
mix a735bfd116 docs: retire the test review plan, keep its durable findings
Every item in TEST_REVIEW_PLAN.md is done or decided, so the working
document goes as its own header instructed. What outlives it moves to
where a later reviewer will actually look:

- TESTS.md gains the -coverpkg command and the 84.4% baseline (per-package
  figures understate the suite), design principle 9 (redundancy is judged
  by comparing coverage profiles, and identical coverage alone is not
  grounds for deletion), a table of the look-alike tests that are kept
  with the reason each survives, and the list of functions deliberately
  at 0%.
- STANDARDS.md's "Intentional behavior" section points at both lists, so
  the mechanism REVIEW.md describes still reaches them.

Dropped as spent: the per-item checklists, the suggested order, and the
model-selection table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:53:13 +03:00
mix 28f0a0d8e2 test: resolve the thin-test item, decline the runner merge
Item 4 of the test-suite review:

- Delete TestEmitWithNoObserversIsNoop (no assertion; ranging a nil slice
  cannot panic) and TestStoreReturnsWiredStore (a getter returning its
  own field).
- Collapse the four TestFilteredJobIndexes* tests into one table-driven
  TestFilteredJobIndexes, matching TestFilterValue above it.
- Replace the TestMainViewBuilds smoke test with
  TestMainViewRecordStartupAddsHistoryRow, which calls the recordStartup
  closure for both wordings run.go selects between and asserts the rows
  reach the History table through its own cell callbacks. Keeps the
  unique coverage the review identified and adds the !windowShown branch.

Item 5 is declined with measurements: the three RunJob tests cost 0.14 s
combined, so merging them saves ~90 ms while forcing their three
fixtures (including the only Manual trigger) into one. The runner
package's runtime is the two timeout tests, not subprocess spawns.

go vet and go test -race pass for src/app and src/ui.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 22:46:26 +03:00
7 changed files with 138 additions and 236 deletions
+4
View File
@@ -66,6 +66,10 @@ change to their shape has to stay compatible on its own.
- **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the - **History tab is session-only.** `JobRuntime.Logs` exists only in memory for the
current process. Log files on disk feed aggregate statistics via `SeedStats` current process. Log files on disk feed aggregate statistics via `SeedStats`
only. See [ARCHITECTURE.md](ARCHITECTURE.md). only. See [ARCHITECTURE.md](ARCHITECTURE.md).
- Several tests share a coverage profile with another test on purpose, and a few
functions sit at 0% on purpose. Both lists live in
[TESTS.md](TESTS.md) — check them before reporting a test as redundant or a
coverage gap as an oversight.
## Out of scope ## Out of scope
+40 -7
View File
@@ -55,6 +55,17 @@ go test -coverprofile=coverage.out ./src/runner
go tool cover -html=coverage.out go tool cover -html=coverage.out
``` ```
Per-package coverage understates the suite, because several packages are
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
```
That figure was 84.4% at the 2026-08-04 review, which is the number to compare
against before concluding that coverage has slipped.
--- ---
## Test Files Overview ## Test Files Overview
@@ -102,7 +113,6 @@ Tests `Service` construction and the state-accessor contract.
|------|---------| |------|---------|
| `TestNewServiceBuildsRuntimePerJob` | Verifies that `NewService` creates a `JobRuntime` entry for every loaded job. | | `TestNewServiceBuildsRuntimePerJob` | Verifies that `NewService` creates a `JobRuntime` entry for every loaded job. |
| `TestJobsReturnsCopy` | Verifies that `Service.Jobs` returns a defensive copy so callers cannot mutate internal state. | | `TestJobsReturnsCopy` | Verifies that `Service.Jobs` returns a defensive copy so callers cannot mutate internal state. |
| `TestStoreReturnsWiredStore` | Verifies that `Service.Store` returns the injected `storage.Store`. |
--- ---
@@ -195,7 +205,6 @@ Tests the event-emission and observer-subscription machinery.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestEmitDeliversToAllObserversInOrder` | Verifies that all registered observers receive emitted events in registration order. | | `TestEmitDeliversToAllObserversInOrder` | Verifies that all registered observers receive emitted events in registration order. |
| `TestEmitWithNoObserversIsNoop` | Verifies that emitting an event with no observers does not panic. |
| `TestObserverCanReadServiceState` | Verifies that an observer called by `emit` can safely read Service state (jobs, runtimes). | | `TestObserverCanReadServiceState` | Verifies that an observer called by `emit` can safely read Service state (jobs, runtimes). |
--- ---
@@ -436,10 +445,7 @@ widgets are assembled.
| `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. | | `TestFilterValue` | Verifies that `filterValue` returns the correct display string for the current folder filter. |
| `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. | | `TestFolderOptionsAlwaysIncludesSentinels` | Verifies that the folder filter list always starts with "All" and "No folder" sentinel entries. |
| `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. | | `TestFolderOptionsAppendsUniqueFolders` | Verifies that folder names from the job list are appended once each, in order, without duplicates. |
| `TestFilteredJobIndexesAll` | Verifies that the "All" filter returns indexes for every job. | | `TestFilteredJobIndexes` | Table: verifies the "All" filter returns every index, a named folder returns only its own jobs, "No folder" matches empty and blank folder fields, and an empty job list yields no indexes. |
| `TestFilteredJobIndexesByNamedFolder` | Verifies that filtering by a named folder returns only jobs in that folder. |
| `TestFilteredJobIndexesNoFolder` | Verifies that the "No folder" filter returns only jobs with an empty folder field. |
| `TestFilteredJobIndexesEmptySlice` | Verifies that filtering an empty job slice returns an empty index list. |
| `TestNextJobListViewFlipsBothWays` | Verifies the density toggle alternates between detailed and compact from either starting value. | | `TestNextJobListViewFlipsBothWays` | Verifies the density toggle alternates between detailed and compact from either starting value. |
| `TestViewToggleTextNamesTheAction` | Verifies the toggle button is labelled with the action it performs, not the state it is in. | | `TestViewToggleTextNamesTheAction` | Verifies the toggle button is labelled with the action it performs, not the state it is in. |
| `TestJobListViewToggleShrinksRowsAndPersists` | End-to-end: one tap shrinks the row height, relabels the button, and reaches the config; tapping back undoes all three. | | `TestJobListViewToggleShrinksRowsAndPersists` | End-to-end: one tap shrinks the row height, relabels the button, and reaches the config; tapping back undoes all three. |
@@ -528,7 +534,7 @@ Tests main view construction with an injected `*app.Service`.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestMainViewFitsTheDefaultWindowSize` | Verifies the assembled content's minimum fits the window size the app asks for, so Fyne never silently widens the window past it. The store's config path is deliberately long, since it was the path label that used to grow the Settings tab. | | `TestMainViewFitsTheDefaultWindowSize` | Verifies the assembled content's minimum fits the window size the app asks for, so Fyne never silently widens the window past it. The store's config path is deliberately long, since it was the path label that used to grow the Settings tab. |
| `TestMainViewBuilds` | Verifies `newMainView` assembles tabs without panic using `fyne.io/fyne/v2/test`. | | `TestMainViewRecordStartupAddsHistoryRow` | Verifies the `recordStartup` closure `newMainView` returns appends the startup receipt to History and redraws the table, with the windowed and tray wordings `run.go` selects between. |
--- ---
@@ -550,6 +556,23 @@ Tests main view construction with an injected `*app.Service`.
8. **Geometry is measured, not eyeballed** — The `ui` tests that build widgets under `test.NewApp()` assert sizes and offsets, and several re-run under a scaled theme. That is what keeps [STANDARDS.md](STANDARDS.md)'s "measure at build time, never a pixel constant" rule enforceable rather than aspirational. 8. **Geometry is measured, not eyeballed** — The `ui` tests that build widgets under `test.NewApp()` assert sizes and offsets, and several re-run under a scaled theme. That is what keeps [STANDARDS.md](STANDARDS.md)'s "measure at build time, never a pixel constant" rule enforceable rather than aspirational.
9. **Redundancy is measured, not read** — Before deleting a test as a duplicate, run both in isolation with `-coverprofile` and compare the profiles. Identical coverage alone is *not* grounds for deletion: several kept tests hit the same statements while asserting genuinely different properties (see the table below). Deletion requires identical coverage **and** assertions that are a subset of the survivor's.
---
## Look-alike tests that are kept
Every pair here has an identical coverage profile, so a redundancy pass will
flag them again. They were measured under principle 9 and kept because the
assertions differ — not because nobody looked.
| Tests | Why both stay |
|-------|---------------|
| `TestSetGlobalPausePersistsToConfigFile` / `TestSetGlobalPauseUpdatesRuntimesAndEmits` | The first asserts the flag reaches `gosentry.json`, which is what makes the pause survive a restart; the second asserts the in-memory runtimes and the emitted event. |
| `TestRunDueQueueDrainsMultipleOverlaps` / `TestRunDueQueueRerunsAfterFinish` | The first drains three queued occurrences rather than one, so it is the test that would catch a drain loop that fires only once. |
| `TestCreateStartupShortcutHandlesCyrillicPath` / `TestCreateStartupShortcutHandlesSpaces` | Non-ASCII paths and paths with spaces are different real-world failure modes for the WScript.Shell COM call. |
| `TestRunJobLogFileAllHeaders` / `TestRunJobRecordFields` / `TestRunJobWritesLogFile` | Three different subjects: the log file's headers, the returned `RunRecord`'s fields, and the log file's name and directory. The fixtures differ too — only `TestRunJobWritesLogFile` runs the `Manual` trigger. Merging them into one `RunJob` call was measured and declined: it saves ~90 ms (the three cost 0.14 s combined; the `runner` package's seconds are `TestRunJobTimesOut` and `TestRunJobZeroTimeoutMeansNoTimeout`, which wait on purpose) and would drop the `Manual` path from the header assertions. |
--- ---
## Remaining Test Coverage Gaps ## Remaining Test Coverage Gaps
@@ -557,3 +580,13 @@ Tests main view construction with an injected `*app.Service`.
- Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end; the `ui` tests assemble views and measure them, but nothing drives a real window. - Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end; the `ui` tests assemble views and measure them, but nothing drives a real window.
- History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [STANDARDS.md](STANDARDS.md)) - History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [STANDARDS.md](STANDARDS.md))
- Fyne's headless driver cannot report a maximized window, which is why window-size persistence stays frozen in [ROADMAP.md](ROADMAP.md) - Fyne's headless driver cannot report a maximized window, which is why window-size persistence stays frozen in [ROADMAP.md](ROADMAP.md)
### Functions deliberately at 0%
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`, `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.
- `app.Service.ShouldNotifyOnFailure` — a getter under the mutex.
-178
View File
@@ -1,178 +0,0 @@
# 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.
-6
View File
@@ -33,12 +33,6 @@ func TestEmitDeliversToAllObserversInOrder(t *testing.T) {
} }
} }
func TestEmitWithNoObserversIsNoop(t *testing.T) {
svc := newTestService(nil)
// Must not panic with an empty observer list.
svc.emit(JobChanged{})
}
// Observers may read Service state from within OnEvent without deadlocking, // Observers may read Service state from within OnEvent without deadlocking,
// because emit is called outside the state lock. // because emit is called outside the state lock.
func TestObserverCanReadServiceState(t *testing.T) { func TestObserverCanReadServiceState(t *testing.T) {
-8
View File
@@ -47,11 +47,3 @@ func TestJobsReturnsCopy(t *testing.T) {
t.Errorf("Service state leaked through Jobs(): name = %q, want %q", again[0].Name, "Original") t.Errorf("Service state leaked through Jobs(): name = %q, want %q", again[0].Name, "Original")
} }
} }
func TestStoreReturnsWiredStore(t *testing.T) {
store := &storage.Store{}
svc := NewService(store, nil)
if svc.Store() != store {
t.Error("Store() did not return the wired store")
}
}
+27 -32
View File
@@ -56,48 +56,43 @@ func TestFolderOptionsAppendsUniqueFolders(t *testing.T) {
} }
} }
func TestFilteredJobIndexesAll(t *testing.T) { func TestFilteredJobIndexes(t *testing.T) {
jobs := []domain.Job{
{Folder: "Maintenance"},
{Folder: ""},
{Folder: "Reports"},
}
got := filteredJobIndexes(jobs, allFolders)
if len(got) != 3 {
t.Errorf("allFolders filter: got %d indexes, want 3", len(got))
}
}
func TestFilteredJobIndexesByNamedFolder(t *testing.T) {
jobs := []domain.Job{ jobs := []domain.Job{
{Folder: "Maintenance"}, // index 0 {Folder: "Maintenance"}, // index 0
{Folder: ""}, // index 1 {Folder: ""}, // index 1 — no folder
{Folder: "Maintenance"}, // index 2 {Folder: "Maintenance"}, // index 2
{Folder: "Reports"}, // index 3 {Folder: "Reports"}, // index 3
{Folder: " "}, // index 4 — blank reads as no folder
} }
got := filteredJobIndexes(jobs, "Maintenance") cases := []struct {
if len(got) != 2 || got[0] != 0 || got[1] != 2 { name string
t.Errorf("Maintenance filter: got %v, want [0 2]", got) jobs []domain.Job
filter string
want []int
}{
{"all folders", jobs, allFolders, []int{0, 1, 2, 3, 4}},
{"named folder", jobs, "Maintenance", []int{0, 2}},
{"no folder", jobs, noFolder, []int{1, 4}},
{"empty job list", nil, allFolders, nil},
}
for _, tc := range cases {
got := filteredJobIndexes(tc.jobs, tc.filter)
if !sameIndexes(got, tc.want) {
t.Errorf("%s: filteredJobIndexes(_, %q) = %v, want %v", tc.name, tc.filter, got, tc.want)
}
} }
} }
func TestFilteredJobIndexesNoFolder(t *testing.T) { func sameIndexes(got, want []int) bool {
jobs := []domain.Job{ if len(got) != len(want) {
{Folder: "Maintenance"}, // index 0 — excluded return false
{Folder: ""}, // index 1 — no folder → included
{Folder: " "}, // index 2 — blank → included
} }
got := filteredJobIndexes(jobs, noFolder) for i := range got {
if len(got) != 2 || got[0] != 1 || got[1] != 2 { if got[i] != want[i] {
t.Errorf("noFolder filter: got %v, want [1 2]", got) return false
} }
}
func TestFilteredJobIndexesEmptySlice(t *testing.T) {
got := filteredJobIndexes(nil, allFolders)
if len(got) != 0 {
t.Errorf("empty job list should return empty indexes, got %v", got)
} }
return true
} }
func TestNextJobListViewFlipsBothWays(t *testing.T) { func TestNextJobListViewFlipsBothWays(t *testing.T) {
+67 -5
View File
@@ -3,12 +3,16 @@ package ui
import ( import (
"path/filepath" "path/filepath"
"testing" "testing"
"time"
"gitea.mixdep.ru/mix/gosentry/src/app" "gitea.mixdep.ru/mix/gosentry/src/app"
"gitea.mixdep.ru/mix/gosentry/src/domain" "gitea.mixdep.ru/mix/gosentry/src/domain"
"gitea.mixdep.ru/mix/gosentry/src/storage" "gitea.mixdep.ru/mix/gosentry/src/storage"
"fyne.io/fyne/v2"
"fyne.io/fyne/v2/container"
"fyne.io/fyne/v2/test" "fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/widget"
) )
// newTestStore builds a Store rooted in a temp directory. It is separate from // newTestStore builds a Store rooted in a temp directory. It is separate from
@@ -69,7 +73,35 @@ func TestMainViewFitsTheDefaultWindowSize(t *testing.T) {
} }
} }
func TestMainViewBuilds(t *testing.T) { // historyTable returns the History tab's table. It is the only widget.Table the
// main view builds, so the search does not need to know the tab order.
func historyTable(t *testing.T, content fyne.CanvasObject) *widget.Table {
t.Helper()
tabs, ok := content.(*container.AppTabs)
if !ok {
t.Fatal("main view is not the expected AppTabs container")
}
for _, item := range tabs.Items {
found := findFirst(item.Content, func(o fyne.CanvasObject) bool {
_, ok := o.(*widget.Table)
return ok
})
if found != nil {
return found.(*widget.Table)
}
}
t.Fatal("main view has no history table")
return nil
}
// TestMainViewRecordStartupAddsHistoryRow covers the second return value of
// newMainView. run.go calls it once per launch with a different windowShown
// flag depending on whether the app started into the tray, and that call is the
// only thing that puts the startup receipt into History — so both the wording
// and the fact that the table is redrawn are worth pinning. Building the full
// tab set and setting it as the window content is a side benefit: no other test
// assembles all three tabs together.
func TestMainViewRecordStartupAddsHistoryRow(t *testing.T) {
testApp := test.NewApp() testApp := test.NewApp()
defer testApp.Quit() defer testApp.Quit()
@@ -80,9 +112,39 @@ func TestMainViewBuilds(t *testing.T) {
defer svc.Stop() defer svc.Stop()
content, recordStartup := newMainView(w, svc) content, recordStartup := newMainView(w, svc)
if content == nil {
t.Fatal("newMainView returned nil content")
}
w.SetContent(content) w.SetContent(content)
recordStartup(0, true)
table := historyTable(t, content)
if rows, _ := table.Length(); rows != 0 {
t.Fatalf("history rows before startup = %d, want 0", rows)
}
recordStartup(1500*time.Millisecond, true)
recordStartup(20*time.Millisecond, false)
rows, _ := table.Length()
if rows != 2 {
t.Fatalf("history rows after two startup records = %d, want 2", rows)
}
// Read the rows back through the table's own cell callbacks, which is what
// the redraw does; a value only in the events slice would not prove the
// table was refreshed with it.
cell := table.CreateCell()
cellText := func(row, col int) string {
table.UpdateCell(widget.TableCellID{Row: row, Col: col}, cell)
return cell.(*widget.Label).Text
}
if got := cellText(0, 2); got != "Application" {
t.Errorf("startup row job = %q, want %q", got, "Application")
}
if got := cellText(0, 3); got != "Started" {
t.Errorf("startup row state = %q, want %q", got, "Started")
}
if got := cellText(0, 4); got != "Window shown in 1.5s" {
t.Errorf("windowed startup detail = %q, want %q", got, "Window shown in 1.5s")
}
if got := cellText(1, 4); got != "Started in tray in 20ms" {
t.Errorf("tray startup detail = %q, want %q", got, "Started in tray in 20ms")
}
} }