From aed83b91b9270137e30fbd54615238ffd05509a8 Mon Sep 17 00:00:00 2001 From: mixeme Date: Wed, 1 Jul 2026 23:14:09 +0300 Subject: [PATCH] docs: add FUTURE_WORK quality standard and close polish gaps Replace CODE_REVIEW.md with a living maturity checklist, document session-only History, inject Service into newMainView for testability, add UI and scheduler regression tests, and fix RunNow error surfacing plus empty jobs view handling. Co-authored-by: Cursor --- README.md | 1 + docs/ARCHITECTURE.md | 9 ++ docs/CODE_REVIEW.md | 53 -------- docs/FUTURE_WORK.md | 112 +++++++++++++++++ docs/TESTS.md | 82 +++++++++++- src/app/format_test.go | 12 ++ src/app/run_test.go | 84 +++++++++++++ src/platform/desktop/desktop_linux_test.go | 62 +++++++++ src/ui/history_view_test.go | 138 +++++++++++++++++++++ src/ui/jobs_view.go | 17 ++- src/ui/jobs_view_helpers.go | 2 +- src/ui/mainwindow.go | 7 +- src/ui/mainwindow_test.go | 56 +++++++++ src/ui/run.go | 10 +- 14 files changed, 576 insertions(+), 69 deletions(-) delete mode 100644 docs/CODE_REVIEW.md create mode 100644 docs/FUTURE_WORK.md create mode 100644 src/platform/desktop/desktop_linux_test.go create mode 100644 src/ui/history_view_test.go create mode 100644 src/ui/mainwindow_test.go diff --git a/README.md b/README.md index 1890d06..e70689d 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ creating, grouping, pausing, running, and monitoring scheduled shell commands. - [Changelog](docs/CHANGELOG.md) - [Roadmap](docs/ROADMAP.md) - [Architecture](docs/ARCHITECTURE.md) +- [Future Work](docs/FUTURE_WORK.md) — quality standard, maturity checklist, open polish - [Development](docs/DEVELOPMENT.md) — build instructions, project layout, dependencies ## Storage diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ff0f247..3328621 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -97,6 +97,9 @@ flowchart LR When a run goroutine completes, `Service` updates the job's runtime (including the statistics aggregate), saves JSON, triggers log cleanup, and emits `RunRecorded`. The UI observer appends the record to the History tab. + History rows exist only for the current process session; restarting the app + clears the table (aggregate stats in the details panel are still seeded from + log files). 7. Autostart: `UpdateSettings` in the Service calls `autostart.Manager.Set`. The Manager @@ -150,6 +153,12 @@ accumulated run history immediately after a restart. Older log files that pre-date the `duration` header are tolerated: the run is counted but the timing is skipped. +`JobRuntime.Logs` (per-run `RunRecord` entries shown in the History tab) is +**session-only**: it is not written to `jobs.json` and is not rebuilt from +`.log` files on startup. Log files on disk feed aggregate counters via +`SeedStats` only. See [FUTURE_WORK.md](FUTURE_WORK.md) for the intentional +trade-off. + ### Persisted global pause `domain.Config` carries a `Paused bool` field (`json:"paused,omitempty"`). diff --git a/docs/CODE_REVIEW.md b/docs/CODE_REVIEW.md deleted file mode 100644 index 8d4e548..0000000 --- a/docs/CODE_REVIEW.md +++ /dev/null @@ -1,53 +0,0 @@ -# GoSentry — Code Review (2026-06-29) - -Версия на момент ревью: **0.11.2** (исправления вошли в **0.11.3**) - -## Итог - -| Критерий | Оценка | -|----------|--------| -| Архитектура | 9/10 | -| Сложность vs масштаб | 8/10 | -| Качество кода | 8/10 | -| Поддерживаемость | 8/10 | -| Логические ошибки | 9/10 (после исправлений) | - -Проект зрелый и поддерживаемый для десктопного планировщика (~59 `.go`-файлов). Архитектура слоистая, core-логика хорошо протестирована. - -## Сильные стороны - -- Single-writer `app.Service` с явным locking contract -- Разделение `domain.Job` (durable) и `domain.JobRuntime` (transient) -- Event-driven UI без обратных вызовов в Fyne под lock -- Portable storage от `os.Executable()` -- Инъекция `runJob` и `scheduler.Clock` в тестах -- Подробная документация (`ARCHITECTURE.md`, inline comments) - -## Найденные проблемы и статус исправлений - -| # | Проблема | Серьёзность | Статус | -|---|----------|-------------|--------| -| 1 | Data race: `store.Paths` в `executeRun` без lock | Высокая | Исправлено | -| 2 | Run стартует при ошибке `SaveJobs` | Средняя | Исправлено | -| 3 | CRUD эмитит events при failed save | Средняя | Исправлено | -| 4 | Overlap queue — только один `Pending` | Средняя | Исправлено (`PendingRuns`) | -| 5 | `time.Now()` vs scheduler clock в `startRunLocked` | Низкая | Исправлено | -| 6 | Silent log write failures | Низкая | Исправлено | -| 7 | Невалидный per-job `overlap_policy` | Низкая | Исправлено | -| 8 | Docs drift (YAML, RunNow/pause) | Низкая | Исправлено | -| 9 | `StartOnly` игнорировал cancel context | Низкая | Исправлено | -| 10 | `SeedStats` коллизия sanitized имён | Низкая | Исправлено (match по `job_id`) | -| 11 | `AvgDurationMS` seed vs live расходились | Низкая | Исправлено (`TimedRunCount`) | -| 12 | Legacy ticket-ссылки в комментариях | Низкая | Исправлено | - -## Намеренное поведение (не баги) - -- `RunNow` разрешён при global pause и для disabled jobs -- Sequential mode — FIFO по порядку в `jobs.json` -- Scheduler tick 1s — sub-second `@every` не поддерживается -- Command timeout 30s — глобальный лимит - -## Рекомендации на будущее - -- UI widget tests или smoke E2E -- Per-job command timeout в конфиге diff --git a/docs/FUTURE_WORK.md b/docs/FUTURE_WORK.md new file mode 100644 index 0000000..d92a5cb --- /dev/null +++ b/docs/FUTURE_WORK.md @@ -0,0 +1,112 @@ +# GoSentry — Future Work + +> Временный документ: цель проекта, стандарт качества и открытые задачи. +> После завершения шлифовки переименуем (например, в `STANDARDS.md`). + +## Зачем этот документ + +**Образцовый проект** — тот, на который можно сослаться как на эталон: архитектура +понятна с первого прочтения, границы пакетов соблюдаются, намеренные компромиссы +задокументированы, поведение воспроизводимо тестами, новый контрибьютор знает +*как* и *почему* писать код здесь. + +Оценки зрелости ниже — **зеркало**, не KPI. Поднимать балл ради балла не имеет +смысла; имеет смысл закрывать пункты чеклиста. + +## Базовая оценка (внутреннее ревью, 2026-06-29) + +| Критерий | Оценка | +|----------|--------| +| Архитектура | 9/10 | +| Сложность vs масштаб | 8/10 | +| Качество кода | 8/10 | +| Поддерживаемость | 8/10 | +| Логические ошибки | 9/10 | + +Ревью проводилось на **0.11.2**; исправления вошли в **0.11.3–0.11.4**. +Текущая версия: `src/app/version.go`. + +## Архитектурные сильные стороны + +- Single-writer `app.Service` с явным locking contract +- Разделение `domain.Job` (durable) и `domain.JobRuntime` (transient) +- Event-driven UI без обратных вызовов в Fyne под lock +- Portable storage от `os.Executable()` +- Инъекция `runJob` и `scheduler.Clock` в тестах +- Подробная документация (`ARCHITECTURE.md`, inline comments) + +## Стандарт качества (обязателен для нового кода) + +- Контракты пакетов — [ARCHITECTURE.md](ARCHITECTURE.md) +- User-facing error → `dialog.ShowError` или History event, не silent `return` +- Pure helpers → unit-тест в том же пакете +- Фикс severity ≥ medium → regression-тест +- Намеренное поведение → §«Намеренное поведение» ниже, не backlog-баг +- UI view-конструкторы принимают `*app.Service`, не вызывают `app.Open()` внутри + +## Чеклист зрелости + +| # | Критерий | Статус | +|---|----------|--------| +| 1 | Контракты пакетов задокументированы и соблюдаются | ✓ | +| 2 | Намеренные trade-off'ы явно записаны | ✓ | +| 3 | Нет silent failures в user-facing путях | ✓ | +| 4 | Pure logic вынесена и покрыта unit-тестами | ✓ | +| 5 | Regression-тест на серьёзные фиксы ревью | ✓ | +| 6 | DI на границе UI↔Service | ✓ | +| 7 | Документация = код | ✓ | +| 8 | Platform-код тестируется по одному образцу | ✓ | +| 9 | Единый стиль ошибок в UI | ✓ | + +## Намеренное поведение (не баги) + +- `RunNow` разрешён при global pause и для disabled jobs +- Sequential mode — FIFO по порядку в `jobs.json` +- Scheduler tick 1s — sub-second `@every` не поддерживается +- Command timeout 30s — глобальный лимит +- **History tab — session-only.** `JobRuntime.Logs` живёт только в памяти + текущей сессии. Файлы в `logs_dir` используются для aggregate stats + (`SeedStats`), не для таблицы History. Подробнее — [ARCHITECTURE.md](ARCHITECTURE.md). + +## Закрытые находки ревью (2026-06-29) + +Историческая справка; не открывать повторно без новых данных. + +| # | Проблема | Серьёзность | Статус | +|---|----------|-------------|--------| +| 1 | Data race: `store.Paths` в `executeRun` без lock | Высокая | Исправлено | +| 2 | Run стартует при ошибке `SaveJobs` | Средняя | Исправлено | +| 3 | CRUD эмитит events при failed save | Средняя | Исправлено | +| 4 | Overlap queue — только один `Pending` | Средняя | Исправлено (`PendingRuns`) | +| 5 | `time.Now()` vs scheduler clock в `startRunLocked` | Низкая | Исправлено | +| 6 | Silent log write failures | Низкая | Исправлено | +| 7 | Невалидный per-job `overlap_policy` | Низкая | Исправлено | +| 8 | Docs drift (YAML, RunNow/pause) | Низкая | Исправлено | +| 9 | `StartOnly` игнорировал cancel context | Низкая | Исправлено | +| 10 | `SeedStats` коллизия sanitized имён | Низкая | Исправлено (match по `job_id`) | +| 11 | `AvgDurationMS` seed vs live расходились | Низкая | Исправлено (`TimedRunCount`) | +| 12 | Legacy ticket-ссылки в комментариях | Низкая | Исправлено | + +## Вне scope (осознанные trade-off'ы) + +| Item | Где зафиксировано | +|------|-------------------| +| History из `.log` | Session-only by design (§выше) | +| Per-job command timeout | [ROADMAP.md](ROADMAP.md) | +| Window size persistence | [ROADMAP.md](ROADMAP.md) (frozen) | +| Column filters в History | [ROADMAP.md](ROADMAP.md) | +| CI coverage gate | [ROADMAP.md](ROADMAP.md) (будущее) | + +## Честно про 10/10 + +Единые **10/10** по всем критериям — неразумная цель для Fyne desktop: CGO, +platform stubs и headless-лимиты GUI объективно добавляют сложность. +**Образцовость ≠ идеальный балл.** Целевое состояние: чеклист зрелости ✓, +проект как reference implementation layered Go desktop app. + +## Связанные документы + +- [ARCHITECTURE.md](ARCHITECTURE.md) — контракты пакетов +- [TESTS.md](TESTS.md) — как и что тестировать +- [ROADMAP.md](ROADMAP.md) — крупные фичи и platform blockers +- [DEVELOPMENT.md](DEVELOPMENT.md) — сборка и layout diff --git a/docs/TESTS.md b/docs/TESTS.md index cf304a4..81dbc8f 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -117,7 +117,7 @@ Tests all mutating operations on the Service, scheduler integration, and setting | `TestRunNowUsesRunnerAndRecords` | Verifies that `RunNow` invokes the runner, records a `RunRecord`, and emits `RunRecorded`. | | `TestRunNowNotFound` | Verifies that `RunNow` returns an error for an unknown job ID. | | `TestRunNowRefusedWhileAlreadyRunning` | Verifies that a second concurrent `RunNow` on the same job is rejected while the first is in progress. | -| `TestRunNowRefusedWhilePaused` | Verifies that `RunNow` is rejected when the global pause flag is set. | +| `TestRunNowAllowedWhilePaused` | Verifies that `RunNow` is allowed when the global pause flag is set (pause stops scheduled runs only). | | `TestRunDueStartsDueJob` | Verifies that `RunDue` launches a job whose next-run time has passed. | | `TestRunDueSkipsJobNotYetDue` | Verifies that `RunDue` does not launch a job that is not yet due. | | `TestRunDueSkipsJobInRunningState` | Verifies that `RunDue` does not start a second concurrent run for an already-running job. | @@ -134,6 +134,30 @@ Tests all mutating operations on the Service, scheduler integration, and setting --- +### src/app/run_test.go + +**Package:** `app` + +Tests overlap policy, sequential execution, run statistics, and scheduler edge cases using injected `runJob` and `primeDue`. + +| Test | Purpose | +|------|---------| +| `TestUpdateStats` | Verifies aggregate duration math on `JobRuntime`. | +| `TestUpdateStatsSkipsZeroDuration` | Verifies zero-duration runs are excluded from averages. | +| `TestRunDueParallelStartsAllDueJobs` | Parallel mode: both due jobs enter the runner before either completes. | +| `TestRunDueSequentialSerializes` | Sequential mode: job 2 waits until job 1 finishes. | +| `TestRunDueSkipDropsOverlap` | Global skip: no second concurrent run, `PendingRuns` stays 0. | +| `TestRunDueQueueRerunsAfterFinish` | Queue: one deferred run after an in-flight finish. | +| `TestRunDueQueueDrainsMultipleOverlaps` | Queue: multiple missed ticks drain as separate runs. | +| `TestRunDuePerJobQueueOverridesGlobalSkip` | Per-job `queue` beats global `skip`. | +| `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. | +| `TestStartRunLockedRollbackOnSaveFailure` | Regression: run does not start when `SaveJobs` fails. | +| `TestRunDueQueueDrainSkippedWhenPaused` | Queued overlaps are not drained while the scheduler is paused. | + +--- + ### src/app/events_test.go **Package:** `app` @@ -162,7 +186,9 @@ Tests display-formatting helpers used by the UI. | `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 that the display index is one-based (job slice index + 1). | +| `TestDisplayIndex` | Verifies the list position of a job index in a filtered index slice. | +| `TestDisplayStats` | Verifies statistics line formatting for the details panel. | +| `TestDisplayOverlapPolicy` | Verifies per-job vs inherited global overlap policy labels. | --- @@ -302,6 +328,20 @@ Tests Linux autostart via XDG Desktop Entry files. --- +### src/platform/desktop/desktop_linux_test.go + +**Location:** `src/platform/desktop/desktop_linux_test.go` +**Build Tags:** `//go:build linux` + +Tests Linux desktop integration (`.desktop` file and icon under XDG data home). + +| Test | Purpose | +|------|---------| +| `TestInstallDesktopIntegrationWritesDesktopAndIcon` | Verifies `.desktop` and PNG icon files are written under `$XDG_DATA_HOME`. | +| `TestQuoteDesktopExecQuotesPath` | Verifies `Exec=` paths with spaces are shell-quoted. | + +--- + ### src/ui/jobs_view_test.go **Package:** `ui` @@ -317,6 +357,37 @@ Tests pure helper functions in the jobs view (no Fyne widget construction). | `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. | +| `TestLastJobLogsCapsAndCopies` | Verifies activity panel cap and defensive copy semantics. | +| `TestLastJobLogsEmpty` | Verifies nil/empty log input returns an empty slice. | +| `TestIndexOfID` | Verifies job lookup by ID returns `-1` when not found. | + +--- + +### src/ui/history_view_test.go + +**Package:** `ui` + +Tests pure History tab helpers (no Fyne widget construction). + +| Test | Purpose | +|------|---------| +| `TestCollectActivityMergesAndSorts` | Verifies per-job logs are merged and sorted by time. | +| `TestCollectActivitySkipsMissingRuntimes` | Verifies missing runtime entries are skipped safely. | +| `TestHistoryCellText` | Verifies table cell text for all columns; empty trigger → `Unknown`. | +| `TestLogFileName` | Verifies log path basename extraction on Windows and Unix paths. | +| `TestNewEventUsesConsistentTimestampShape` | Verifies UI events use the same timestamp layout as run records. | + +--- + +### src/ui/mainwindow_test.go + +**Package:** `ui` + +Smoke test for main view construction with an injected `*app.Service`. + +| Test | Purpose | +|------|---------| +| `TestMainViewBuilds` | Verifies `newMainView` assembles tabs without panic using `fyne.io/fyne/v2/test`. | --- @@ -334,9 +405,12 @@ Tests pure helper functions in the jobs view (no Fyne widget construction). 6. **Start-Only Mode** — Special handling for long-running processes that should be launched but not waited on, tested separately from normal execution flow. +7. **Regression on serious fixes** — Any fix from an internal review with severity ≥ medium gets a targeted regression test (see `run_test.go` for examples). + --- ## Remaining Test Coverage Gaps -- GUI integration tests — Fyne widget interaction is not yet tested end-to-end -- Job history on-disk retrieval — RunRecord file reading is not covered +- Full GUI E2E — tab navigation, dialog flows, and native file pickers are not exercised end-to-end +- History is session-only by design — `.log` files seed aggregate stats only, not the History table (see [FUTURE_WORK.md](FUTURE_WORK.md)) +- `layout.go` custom layouts — optional Fyne `test.NewApp()` coverage when CGO is available in CI diff --git a/src/app/format_test.go b/src/app/format_test.go index 2b143ed..f0e6516 100644 --- a/src/app/format_test.go +++ b/src/app/format_test.go @@ -151,3 +151,15 @@ func TestEventLine(t *testing.T) { t.Errorf("EventLine blank trigger = %q, want %q", got, want) } } + +func TestDisplayOverlapPolicy(t *testing.T) { + global := domain.OverlapPolicyQueue + jobOwn := domain.Job{OverlapPolicy: string(domain.OverlapPolicySkip)} + if got, want := DisplayOverlapPolicy(jobOwn, global), "skip"; got != want { + t.Errorf("per-job policy = %q, want %q", got, want) + } + inherit := domain.Job{OverlapPolicy: ""} + if got, want := DisplayOverlapPolicy(inherit, global), "queue (global default)"; got != want { + t.Errorf("inherited policy = %q, want %q", got, want) + } +} diff --git a/src/app/run_test.go b/src/app/run_test.go index c0a1300..b833bbf 100644 --- a/src/app/run_test.go +++ b/src/app/run_test.go @@ -2,6 +2,7 @@ package app import ( "context" + "os" "sync/atomic" "testing" "time" @@ -532,3 +533,86 @@ func TestRunNowSequentialGuard(t *testing.T) { } waitRecord(t, done) } + +// TestStartRunLockedRollbackOnSaveFailure is a regression test for CODE_REVIEW +// finding #2: a run must not start when persisting the Running state fails. +func TestStartRunLockedRollbackOnSaveFailure(t *testing.T) { + svc := newTempService(t, []domain.Job{{ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true}}) + if err := svc.store.SaveJobs(svc.jobs); err != nil { + t.Fatalf("seed jobs.json: %v", err) + } + if err := os.Chmod(svc.store.Paths.JobsPath, 0o444); err != nil { + t.Fatalf("chmod jobs.json: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(svc.store.Paths.JobsPath, 0o644) }) + + var started int32 + svc.runJob = func(_ context.Context, job *domain.Job, _ string, _ string) (domain.RunRecord, error) { + atomic.AddInt32(&started, 1) + return domain.RunRecord{Time: "t", JobID: job.ID, JobName: job.Name, State: "OK"}, nil + } + + if err := svc.RunNow(1); err == nil { + t.Fatal("expected RunNow to fail when jobs.json is not writable") + } + if atomic.LoadInt32(&started) != 0 { + t.Error("run goroutine must not start when SaveJobs fails") + } + if rt := svc.Runtime(1); rt == nil || rt.LastState == "Running" { + t.Errorf("runtime should roll back from Running, got %+v", rt) + } +} + +// TestRunDueQueueDrainSkippedWhenPaused verifies that queued overlap runs are not +// drained while the scheduler is globally paused. +func TestRunDueQueueDrainSkippedWhenPaused(t *testing.T) { + svc := newQueueService(t, domain.ExecutionModeParallel, domain.OverlapPolicyQueue, []domain.Job{ + {ID: 1, Name: "A", Schedule: "@every 1h", Command: "echo", Enabled: true}, + }) + + entered := make(chan int, 2) + release := make(chan struct{}) + var calls int32 + svc.runJob = func(_ context.Context, job *domain.Job, _ string, _ string) (domain.RunRecord, error) { + atomic.AddInt32(&calls, 1) + entered <- job.ID + <-release + return domain.RunRecord{Time: "t", JobID: job.ID, JobName: job.Name, State: "OK"}, 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("expected one queued overlap, PendingRuns = %d", pending) + } + + if err := svc.SetGlobalPause(true); err != nil { + t.Fatalf("SetGlobalPause: %v", err) + } + + close(release) + waitRecord(t, done) + expectNoEntry(t, entered) + + svc.mu.Lock() + pending = svc.runtimes[1].PendingRuns + svc.mu.Unlock() + if pending != 1 { + t.Errorf("paused scheduler must not drain queue, PendingRuns = %d, want 1", pending) + } + if got := atomic.LoadInt32(&calls); got != 1 { + t.Errorf("runner called %d time(s), want 1", got) + } +} diff --git a/src/platform/desktop/desktop_linux_test.go b/src/platform/desktop/desktop_linux_test.go new file mode 100644 index 0000000..b8a7015 --- /dev/null +++ b/src/platform/desktop/desktop_linux_test.go @@ -0,0 +1,62 @@ +//go:build linux + +package desktop + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestInstallDesktopIntegrationWritesDesktopAndIcon(t *testing.T) { + dataHome := t.TempDir() + t.Setenv("XDG_DATA_HOME", dataHome) + + appID := "ru.mixeme.gosentry.desktop" + executable := filepath.Join(dataHome, "bin", "gosentry") + icon := []byte{0x89, 0x50, 0x4e, 0x47} // PNG magic prefix is enough for file presence + + iconPath, err := InstallDesktopIntegration(appID, executable, icon) + if err != nil { + t.Fatalf("InstallDesktopIntegration: %v", err) + } + + if _, err := os.Stat(iconPath); err != nil { + t.Fatalf("icon file: %v", err) + } + iconData, err := os.ReadFile(iconPath) + if err != nil { + t.Fatalf("read icon: %v", err) + } + if string(iconData) != string(icon) { + t.Fatalf("icon bytes mismatch") + } + + desktopPath := filepath.Join(dataHome, "applications", appID+".desktop") + data, err := os.ReadFile(desktopPath) + if err != nil { + t.Fatalf("read desktop entry: %v", err) + } + text := string(data) + if !strings.Contains(text, "Name=GoSentry") { + t.Fatalf("desktop entry missing Name: %s", text) + } + if !strings.Contains(text, "StartupWMClass="+appID) { + t.Fatalf("desktop entry missing WM class: %s", text) + } + wantExec := "Exec=" + quoteDesktopExec(executable) + if !strings.Contains(text, wantExec) { + t.Fatalf("desktop entry exec = %s, want substring %q", text, wantExec) + } + if !strings.Contains(text, "Icon="+iconPath) { + t.Fatalf("desktop entry missing Icon path: %s", text) + } +} + +func TestQuoteDesktopExecQuotesPath(t *testing.T) { + got := quoteDesktopExec("/opt/Go Sentry/gosentry") + if got != `"/opt/Go Sentry/gosentry"` { + t.Errorf("quoteDesktopExec = %q", got) + } +} diff --git a/src/ui/history_view_test.go b/src/ui/history_view_test.go new file mode 100644 index 0000000..55e6f18 --- /dev/null +++ b/src/ui/history_view_test.go @@ -0,0 +1,138 @@ +package ui + +import ( + "testing" + "time" + + "gitea.mixdep.ru/mix/gosentry/src/domain" + + "fyne.io/fyne/v2/widget" +) + +func TestLastJobLogsCapsAndCopies(t *testing.T) { + logs := []event{ + {Time: "1", JobName: "a"}, + {Time: "2", JobName: "b"}, + {Time: "3", JobName: "c"}, + {Time: "4", JobName: "d"}, + } + got := lastJobLogs(logs) + if len(got) != maxJobActivityRows { + t.Fatalf("len = %d, want %d", len(got), maxJobActivityRows) + } + for i, want := range []string{"1", "2", "3"} { + if got[i].Time != want { + t.Errorf("got[%d].Time = %q, want %q", i, got[i].Time, want) + } + } + logs[0].Time = "mutated" + if got[0].Time == "mutated" { + t.Error("lastJobLogs must return a defensive copy") + } +} + +func TestLastJobLogsEmpty(t *testing.T) { + if got := lastJobLogs(nil); len(got) != 0 { + t.Errorf("nil input: got %v, want empty", got) + } +} + +func TestIndexOfID(t *testing.T) { + jobs := []job{ + {ID: 10, Name: "A"}, + {ID: 20, Name: "B"}, + } + if got := indexOfID(jobs, 20); got != 1 { + t.Errorf("found: got %d, want 1", got) + } + if got := indexOfID(jobs, 99); got != -1 { + t.Errorf("missing: got %d, want -1", got) + } + if got := indexOfID(nil, 1); got != -1 { + t.Errorf("empty slice: got %d, want -1", got) + } +} + +func TestCollectActivityMergesAndSorts(t *testing.T) { + jobs := []job{ + {ID: 1, Name: "A"}, + {ID: 2, Name: "B"}, + } + runtimes := map[int]*domain.JobRuntime{ + 1: {Logs: []domain.RunRecord{{Time: "2026-01-02 10:00:00", JobID: 1}}}, + 2: {Logs: []domain.RunRecord{{Time: "2026-01-01 09:00:00", JobID: 2}}}, + } + got := collectActivity(jobs, runtimes) + if len(got) != 2 { + t.Fatalf("len = %d, want 2", len(got)) + } + if got[0].Time != "2026-01-01 09:00:00" || got[1].Time != "2026-01-02 10:00:00" { + t.Errorf("sort order = %v, want ascending by Time", got) + } +} + +func TestCollectActivitySkipsMissingRuntimes(t *testing.T) { + jobs := []job{{ID: 1, Name: "A"}} + if got := collectActivity(jobs, nil); len(got) != 0 { + t.Errorf("nil runtimes: got %v, want empty", got) + } +} + +func TestHistoryCellText(t *testing.T) { + events := []event{{ + Time: "2026-06-01 12:00:00", + Trigger: "", + JobName: "Job", + State: "OK", + Detail: "done", + LogFile: `/logs/20260601-120000_Job.log`, + }} + cases := []struct { + col int + want string + }{ + {0, "2026-06-01 12:00:00"}, + {1, "Unknown"}, + {2, "Job"}, + {3, "OK"}, + {4, "done"}, + {5, "20260601-120000_Job.log"}, + } + for _, tc := range cases { + got := historyCellText(widget.TableCellID{Row: 0, Col: tc.col}, events) + if got != tc.want { + t.Errorf("col %d: got %q, want %q", tc.col, got, tc.want) + } + } + if got := historyCellText(widget.TableCellID{Row: -1, Col: 0}, events); got != "" { + t.Errorf("header row: got %q, want empty", got) + } + if got := historyCellText(widget.TableCellID{Row: 99, Col: 0}, events); got != "" { + t.Errorf("out of range row: got %q, want empty", got) + } +} + +func TestLogFileName(t *testing.T) { + cases := []struct{ path, want string }{ + {"", ""}, + {" ", ""}, + {`C:\logs\run.log`, "run.log"}, + {"/var/logs/2026/job.log", "job.log"}, + {"plain.log", "plain.log"}, + } + for _, tc := range cases { + if got := logFileName(tc.path); got != tc.want { + t.Errorf("logFileName(%q) = %q, want %q", tc.path, got, tc.want) + } + } +} + +func TestNewEventUsesConsistentTimestampShape(t *testing.T) { + ev := newEvent(1, "Job", "OK", "detail") + if _, err := time.Parse("2006-01-02 15:04:05", ev.Time); err != nil { + t.Errorf("timestamp %q is not in expected layout: %v", ev.Time, err) + } + if ev.Trigger != "UI" || ev.JobID != 1 || ev.JobName != "Job" { + t.Errorf("unexpected event fields: %+v", ev) + } +} diff --git a/src/ui/jobs_view.go b/src/ui/jobs_view.go index c8cd941..65eae16 100644 --- a/src/ui/jobs_view.go +++ b/src/ui/jobs_view.go @@ -65,11 +65,19 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } selected := 0 + if len(jobs) == 0 { + selected = -1 + } selectedFolder := allFolders schedulerPaused := svc.Store().Config.Paused filteredJobs := filteredJobIndexes(jobs, selectedFolder) - dp := newDetailsPanel(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy) + dp := newDetailsPanel(job{}, &domain.JobRuntime{}, svc.Store().Config.OverlapPolicy) + if selected >= 0 { + dp.update(jobs[selected], runtimeFor(selected), svc.Store().Config.OverlapPolicy) + } else { + dp.clear() + } updateDetails := func(index int) { if index < 0 || index >= len(jobs) { @@ -126,7 +134,9 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } updateDetails(filteredJobs[id]) } - list.Select(selected) + if len(filteredJobs) > 0 && selected >= 0 { + list.Select(app.DisplayIndex(filteredJobs, selected)) + } folderSelect = widget.NewSelect(folderOptions(jobs), func(value string) { if value == "" { @@ -193,9 +203,8 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } // A manual run is allowed even while the scheduler is paused: pause only // stops automatic scheduled runs, not the user's explicit "Run now". - // RunNow still refuses an already-running job (it returns an error); the UI - // has always ignored that case silently, so the run simply does not start. if err := svc.RunNow(jobs[selected].ID); err != nil { + dialog.ShowError(err, w) return } list.Refresh() diff --git a/src/ui/jobs_view_helpers.go b/src/ui/jobs_view_helpers.go index 2aff8bf..31007e6 100644 --- a/src/ui/jobs_view_helpers.go +++ b/src/ui/jobs_view_helpers.go @@ -53,5 +53,5 @@ func indexOfID(jobs []job, id int) int { return index } } - return 0 + return -1 } diff --git a/src/ui/mainwindow.go b/src/ui/mainwindow.go index 131e5e0..286bf51 100644 --- a/src/ui/mainwindow.go +++ b/src/ui/mainwindow.go @@ -10,7 +10,6 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/container" "fyne.io/fyne/v2/theme" - "fyne.io/fyne/v2/widget" ) // The UI package aliases domain types to keep widget callbacks short. The actual @@ -19,11 +18,7 @@ import ( type job = domain.Job type event = domain.RunRecord -func newMainView(w fyne.Window) (fyne.CanvasObject, func(time.Duration, bool)) { - svc, err := app.Open() - if err != nil { - return container.NewPadded(widget.NewLabel("Failed to load GoSentry configuration: " + err.Error())), func(time.Duration, bool) {} - } +func newMainView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func(time.Duration, bool)) { svc.InstallDesktopIcon(appID, assets.IconBytes()) // Build the initial event history from the current runtime state. Jobs and diff --git a/src/ui/mainwindow_test.go b/src/ui/mainwindow_test.go new file mode 100644 index 0000000..9d0241d --- /dev/null +++ b/src/ui/mainwindow_test.go @@ -0,0 +1,56 @@ +package ui + +import ( + "path/filepath" + "testing" + + "gitea.mixdep.ru/mix/gosentry/src/app" + "gitea.mixdep.ru/mix/gosentry/src/domain" + "gitea.mixdep.ru/mix/gosentry/src/storage" + + "fyne.io/fyne/v2/test" +) + +func newTestService(t *testing.T) *app.Service { + t.Helper() + dir := t.TempDir() + store := &storage.Store{ + Paths: storage.Paths{ + ExecutablePath: filepath.Join(dir, "gosentry"), + AppDir: dir, + ConfigPath: filepath.Join(dir, "gosentry.json"), + JobsDir: dir, + JobsPath: filepath.Join(dir, "jobs.json"), + LogsDir: filepath.Join(dir, "logs"), + }, + Config: domain.Config{ + JobsDir: ".", + LogsDir: "logs", + MaxLogFiles: 100, + MaxLogAgeDays: 30, + ExecutionMode: domain.ExecutionModeParallel, + OverlapPolicy: domain.OverlapPolicySkip, + KeepRunningInTray: true, + NotifyOnFailure: true, + }, + } + return app.NewService(store, nil) +} + +func TestMainViewBuilds(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + + w := testApp.NewWindow("test") + defer w.Close() + + svc := newTestService(t) + defer svc.Stop() + + content, recordStartup := newMainView(w, svc) + if content == nil { + t.Fatal("newMainView returned nil content") + } + w.SetContent(content) + recordStartup(0, true) +} diff --git a/src/ui/run.go b/src/ui/run.go index 1ccf0a8..dcf898e 100644 --- a/src/ui/run.go +++ b/src/ui/run.go @@ -9,6 +9,8 @@ import ( "fyne.io/fyne/v2" fyneapp "fyne.io/fyne/v2/app" + "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/widget" ) const appID = "ru.mixeme.gosentry.desktop" @@ -52,7 +54,13 @@ func Run(startInTray bool) { winW := float32(prefs.FloatWithFallback("window.width", 1024)) winH := float32(prefs.FloatWithFallback("window.height", 660)) w.Resize(fyne.NewSize(winW, winH)) - content, recordStartup := newMainView(w) + svc, err := app.Open() + if err != nil { + w.SetContent(container.NewPadded(widget.NewLabel("Failed to load GoSentry configuration: " + err.Error()))) + a.Run() + return + } + content, recordStartup := newMainView(w, svc) w.SetContent(content) serveSingleInstance(instanceListener, w) if startInTray {