Compare commits
6 Commits
v1.0.3
...
462752f995
| Author | SHA1 | Date | |
|---|---|---|---|
| 462752f995 | |||
| ef6902d65c | |||
| 520a7ef98b | |||
| 0038975adc | |||
| f653b1e484 | |||
| 4c49104cce |
@@ -9,6 +9,7 @@ Project notes:
|
||||
- [Changelog](docs/CHANGELOG.md)
|
||||
- [Roadmap](docs/ROADMAP.md)
|
||||
- [Architecture](docs/ARCHITECTURE.md)
|
||||
- [Refactoring plan](docs/REFACTORING.md)
|
||||
|
||||
## Features
|
||||
|
||||
|
||||
@@ -0,0 +1,304 @@
|
||||
# GoSentry Refactoring Plan
|
||||
|
||||
Status: proposed — not yet started.
|
||||
Goal: make the codebase **solid**, **comprehensive**, and **human-readable / maintainable**
|
||||
without changing observable behavior.
|
||||
|
||||
This document is the single source of truth for the refactor. It records the
|
||||
target architecture, the rationale, and a sequence of small, independently
|
||||
reviewable tasks. Each task lists the recommended agent model and effort level.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why refactor
|
||||
|
||||
The application works and is well-commented, but its structure does not scale:
|
||||
|
||||
| # | Problem | Impact |
|
||||
|---|---------|--------|
|
||||
| 1 | `src/gui/app.go` is a 1,057-line monolith | Nothing can be found, reused, or tested in isolation |
|
||||
| 2 | `src/core` is one flat package mixing 7 concerns | No boundaries; everything can call everything |
|
||||
| 3 | **Shared mutable `*[]Job`** between GUI and `Scheduler` | GUI mutates the slice with no lock; scheduler locks the same slice → data race |
|
||||
| 4 | `onChange` mutates Fyne widgets **from the scheduler goroutine** | Latent crash/corruption — Fyne requires UI updates on the main thread |
|
||||
| 5 | `Job` mixes durable config and runtime state (`yaml:"-"` fields) | The "noise" the model fights to exclude lives in the same struct |
|
||||
| 6 | Errors swallowed everywhere (`_ = store.SaveJobs(...)`) | Save failures are invisible to the user |
|
||||
| 7 | No service/controller layer; GUI reaches into `store.Paths`, drives scheduler directly | Business logic is tangled into widget callbacks |
|
||||
| 8 | Schedule strings re-parsed every tick; no `Schedule` value type | Validation scattered; no single source of truth |
|
||||
| 9 | Tests only cover `core`; GUI and orchestration untestable | Documented gap in `docs/TESTS.md` |
|
||||
|
||||
> Note on layout: the project intentionally **keeps the `src/` directory**. The
|
||||
> `src/` → `internal/` move was considered and rejected — it is cosmetic for a
|
||||
> non-imported desktop app and not worth the import-path churn. All packages
|
||||
> below live under `src/`.
|
||||
|
||||
---
|
||||
|
||||
## 2. Target architecture
|
||||
|
||||
The central change is to **insert an application-service layer** that owns all
|
||||
state and exposes intent-based methods. This turns the UI into a thin view and
|
||||
the core packages into stateless engines, dissolving problems 3, 4, 6, and 7.
|
||||
|
||||
```
|
||||
┌──────────────┐ intents ┌─────────────────┐ calls ┌──────────────┐
|
||||
│ ui (Fyne) │ ───────────▶ │ app.Service │ ─────────▶ │ core engines │
|
||||
│ thin views │ ◀─────────── │ (sole owner of │ │ scheduler / │
|
||||
│ fyne.Do only │ events │ state + mutex) │ ◀───────── │ runner / │
|
||||
└──────────────┘ └─────────────────┘ records │ storage │
|
||||
└──────────────┘
|
||||
```
|
||||
|
||||
- **One writer.** `app.Service` holds the job list + runtime state behind a
|
||||
mutex. The UI never mutates state directly — it calls `CreateJob`, `RunNow`,
|
||||
`SetGlobalPause`, etc.
|
||||
- **Events flow back** through an observer interface. The UI's listener is the
|
||||
*only* place that touches widgets, and it marshals onto the main thread with
|
||||
`fyne.Do`.
|
||||
- **Core engines are stateless / injected** — scheduler and runner operate on
|
||||
data passed in, not a shared slice.
|
||||
|
||||
### 2.1 Package layout (all under `src/`)
|
||||
|
||||
```
|
||||
cmd/gosentry/
|
||||
main.go # flag parse → ui.Run
|
||||
|
||||
src/
|
||||
domain/ # pure types, zero external deps
|
||||
job.go # Job (durable config only — no yaml:"-")
|
||||
runtime.go # JobRuntime (LastRun/NextRun/State/Output/Logs)
|
||||
record.go # RunRecord
|
||||
config.go # Config + StartInTrayArgument
|
||||
schedule.go # Schedule value object: Parse / Validate / Next()
|
||||
|
||||
storage/ # persistence + path resolution + migration
|
||||
store.go # Load/SaveConfig, Load/SaveJobs
|
||||
paths.go # ResolvePaths
|
||||
yaml.go # writeYAML helper
|
||||
migration.go # pysentry → gosentry legacy handling
|
||||
|
||||
scheduler/
|
||||
scheduler.go # timing loop; drives Service via callbacks
|
||||
clock.go # Clock interface (real + fake for tests)
|
||||
|
||||
runner/
|
||||
runner.go # RunJob orchestration
|
||||
invocation.go # build exec.Cmd (shared)
|
||||
invocation_windows.go # cmd.exe quoting
|
||||
invocation_other.go # sh -c
|
||||
exitcodes.go # parse / accept success codes
|
||||
logfile.go # writeRunLog + sanitizeFileName
|
||||
cleanup.go # CleanupLogs
|
||||
|
||||
platform/
|
||||
winproc/ # hidden-window helper shared by runner + autostart
|
||||
winproc_windows.go # CREATE_NO_WINDOW / HideWindow
|
||||
winproc_other.go # no-op
|
||||
autostart/
|
||||
autostart.go # Manager interface + Status type
|
||||
windows.go linux.go other.go
|
||||
desktop/
|
||||
desktop_linux.go other.go
|
||||
|
||||
app/
|
||||
service.go # owns state; CreateJob/UpdateJob/Delete/RunNow/...
|
||||
events.go # Event types + Observer registration
|
||||
format.go # display strings (moved out of GUI)
|
||||
|
||||
ui/ # renamed from src/gui; thin Fyne views
|
||||
run.go # Run(): lifecycle, window, tray wiring
|
||||
mainwindow.go # tab assembly + event listener (fyne.Do)
|
||||
jobs_view.go # list + details panel + toolbar
|
||||
job_dialog.go # new/edit form
|
||||
history_view.go # history table
|
||||
settings_view.go # settings form
|
||||
tray.go # system tray
|
||||
singleinstance.go # localhost IPC
|
||||
layout.go # minWidthLayout
|
||||
```
|
||||
|
||||
Import paths follow the existing convention, e.g.
|
||||
`gitea.mixdep.ru/mix/gosentry/src/domain`,
|
||||
`gitea.mixdep.ru/mix/gosentry/src/app`.
|
||||
|
||||
### 2.2 Dependency direction (must stay acyclic)
|
||||
|
||||
```
|
||||
domain ← (no deps)
|
||||
storage ← domain
|
||||
runner ← domain, platform/winproc
|
||||
scheduler← domain
|
||||
app ← domain, storage, scheduler, runner
|
||||
ui ← app, domain (Fyne)
|
||||
platform/autostart, platform/desktop ← (own deps; winproc for windows)
|
||||
cmd ← ui
|
||||
```
|
||||
|
||||
### 2.3 Key design decisions
|
||||
|
||||
1. **Split durable vs. runtime in the domain.** `domain.Job` becomes pure YAML
|
||||
config (no `yaml:"-"`). Runtime state moves to `domain.JobRuntime`, held by
|
||||
the service keyed by job ID. (Resolves #5.)
|
||||
2. **`Schedule` value object.** `schedule.Parse(string) (Schedule, error)`
|
||||
validates once and exposes `Next(time.Time)`. (Resolves #8.)
|
||||
3. **Autostart behind a `Manager` interface**, selected per platform — mockable,
|
||||
no package-level functions.
|
||||
4. **Injectable `Clock`** in the scheduler → deterministic tests.
|
||||
5. **Errors surface to the UI.** Service methods return errors; status bar shows
|
||||
them. No more `_ =` on saves. (Resolves #6.)
|
||||
6. **Thread-safety contract:** core engines never import Fyne; the UI listener is
|
||||
the sole widget mutator and always wraps updates in `fyne.Do`. (Resolves #4.)
|
||||
|
||||
---
|
||||
|
||||
## 3. Task sequence
|
||||
|
||||
Tasks are ordered so the tree **compiles and all tests pass after every task**.
|
||||
Each task is a small, reviewable unit.
|
||||
|
||||
**Model guidance**
|
||||
- `haiku` — mechanical moves, renames, no judgment required.
|
||||
- `sonnet` — localized logic changes with clear scope.
|
||||
- `opus` — architecture-shaping work (new layers, concurrency, public APIs).
|
||||
|
||||
**Effort guidance** — reasoning depth, not size: `low` / `medium` / `high`.
|
||||
|
||||
### Phase 0 — Safety net
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T0.1 | Add `scripts/test.sh` + `.bat` running `go vet ./...` and `go test -race ./...`. Document in `docs/TESTS.md`. | haiku | low |
|
||||
| T0.2 | Add characterization tests that pin current behavior at seams to be moved: store load→save round-trip, scheduler `nextRunTime`, end-to-end `RunJob` log output. (Some exist; fill gaps.) | sonnet | medium |
|
||||
|
||||
### Phase 1 — Split the flat `core` package (no logic change)
|
||||
|
||||
Mechanical moves + import fixes only. Behavior identical.
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T1.1 | Create `src/domain`; move `Job`, `RunRecord`, `Config`, `JobsFile`, `StartInTrayArgument` from `model.go`. Keep `yaml:"-"` fields for now (split happens in Phase 2). Update all references. | sonnet | medium |
|
||||
| T1.2 | Create `src/platform/winproc`; move `configureHiddenWindow` + hidden-window flags out of `runner_windows.go` / `runner_other.go`. This breaks the future autostart→runner coupling early. | sonnet | medium |
|
||||
| T1.3 | Create `src/runner`; move `runner.go`, `runner_windows.go`, `runner_other.go`, `runner_test.go`. Point at `winproc`. Split helpers into `invocation*.go`, `exitcodes.go`, `logfile.go`, `cleanup.go` as the file moves. | sonnet | medium |
|
||||
| T1.4 | Create `src/scheduler`; move `scheduler.go`, `scheduler_test.go`. Still takes `*[]domain.Job` for now. | sonnet | medium |
|
||||
| T1.5 | Create `src/storage`; move `store.go`, `paths.go`, `store_test.go`. | sonnet | medium |
|
||||
| T1.6 | Create `src/platform/autostart`; move `autostart_*.go` + tests. Point at `winproc`. | sonnet | medium |
|
||||
| T1.7 | Create `src/platform/desktop`; move `desktop_linux.go`, `desktop_other.go`. | haiku | low |
|
||||
| T1.8 | Delete the now-empty `src/core`; run full build + tests on both platforms (or with build tags) to confirm parity. | haiku | low |
|
||||
|
||||
### Phase 2 — Domain cleanup
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T2.1 | Add `src/domain/schedule.go`: `Schedule` value object with `Parse`, `Validate`, `Next(time.Time)`. Unit-test it. Keep `nextRunTime` as a thin wrapper initially. | opus | high |
|
||||
| T2.2 | Migrate `scheduler` to use `Schedule` (parse on load/edit, not per tick). Remove duplicated parsing. | sonnet | medium |
|
||||
| T2.3 | Split `domain.Job` (durable) from `domain.JobRuntime` (transient). Remove all `yaml:"-"` fields and `nextDue` from `Job`. Add `runtime.go`. | opus | high |
|
||||
| T2.4 | Update `storage`: load/save only `Job`; move runtime initialization out of `normalizeJobs` into a `domain.NewRuntime(job)` constructor. Update round-trip tests. | sonnet | medium |
|
||||
|
||||
> After Phase 2 the scheduler and GUI still share state; the `Job`/`JobRuntime`
|
||||
> split is wired through temporary glue. Phase 3 removes the sharing.
|
||||
|
||||
### Phase 3 — Application service layer
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T3.1 | Create `src/app/service.go`: `Service` owning `[]domain.Job` + `map[int]*domain.JobRuntime` behind a `sync.Mutex`. Constructor wires `storage`. | opus | high |
|
||||
| T3.2 | Add `src/app/events.go`: `Event` types (job changed, run recorded, scheduler state) + `Observer` registration. Single-threaded dispatch contract documented. | opus | high |
|
||||
| T3.3 | Move state-mutating operations into the service: `CreateJob`, `UpdateJob`, `DeleteJob`, `SetEnabled`, `RunNow`, `SetGlobalPause`, `UpdateSettings`. Each returns `error`. | opus | high |
|
||||
| T3.4 | Convert `scheduler` to operate through the service (no `*[]Job`). Scheduler asks the service for due jobs and reports records back; service is the sole writer. Inject `Clock`. | opus | high |
|
||||
| T3.5 | Move display/format helpers (`displayFolder`, `displayArguments`, `displayRunMode`, `statusText`, …) from GUI into `src/app/format.go`. | haiku | low |
|
||||
| T3.6 | Add `src/app` unit tests (no Fyne): create/edit/delete, enable/pause, global pause, run-now path with a fake runner + fake clock. Big coverage win. | opus | high |
|
||||
|
||||
### Phase 4 — Carve up the GUI
|
||||
|
||||
Rename `src/gui` → `src/ui` and break `app.go` into focused files. The UI now
|
||||
talks only to `app.Service` and reacts to events via `fyne.Do`.
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T4.1 | Rename package `gui` → `ui`; split lifecycle into `run.go` + `mainwindow.go`. Wire the event listener and route every widget update through `fyne.Do`. (Resolves #4.) | opus | high |
|
||||
| T4.2 | Extract `jobs_view.go` (list + details + toolbar), driven by service calls + events. | sonnet | medium |
|
||||
| T4.3 | Extract `job_dialog.go`; validate schedule via `domain.Schedule.Validate`. | sonnet | medium |
|
||||
| T4.4 | Extract `history_view.go`. | sonnet | medium |
|
||||
| T4.5 | Extract `settings_view.go`; surface save/autostart/cleanup errors to the status label. (Resolves #6 in UI.) | sonnet | medium |
|
||||
| T4.6 | Extract `tray.go`, `singleinstance.go`, `layout.go`. | haiku | low |
|
||||
| T4.7 | Confirm `app.go` is gone and `ui` imports only `app` + `domain` + Fyne. Manual smoke test on each platform. | sonnet | medium |
|
||||
|
||||
### Phase 5 — Hardening & docs
|
||||
|
||||
| Task | Description | Model | Effort |
|
||||
|------|-------------|-------|--------|
|
||||
| T5.1 | Replace remaining `_ = ...Save...` with propagated/surfaced errors across service + storage. | sonnet | medium |
|
||||
| T5.2 | Introduce `autostart.Manager` interface + per-platform impls; inject into the service instead of calling package funcs. | sonnet | medium |
|
||||
| T5.3 | Fill documented test gaps: folder filtering, log cleanup (count + age), settings persistence/migration, concurrent run prevention. | sonnet | high |
|
||||
| T5.4 | Run `go test -race ./...` clean. Confirm no data race remains. | haiku | low |
|
||||
| T5.5 | Update `docs/ARCHITECTURE.md`, `docs/TESTS.md`, and the README "Project Layout" section to the new structure. | sonnet | medium |
|
||||
|
||||
---
|
||||
|
||||
## 3.1 Task completion checklist
|
||||
|
||||
Track progress here. Mark tasks complete as they land and pass review.
|
||||
|
||||
### Phase 0 — Safety net
|
||||
- [x] T0.1 — Add test script + `go vet` + `go test -race`
|
||||
- [x] T0.2 — Add characterization tests
|
||||
|
||||
### Phase 1 — Split flat `core` package
|
||||
- [ ] T1.1 — Create `src/domain`; move Job/RunRecord/Config/etc
|
||||
- [ ] T1.2 — Create `src/platform/winproc`; move `configureHiddenWindow`
|
||||
- [ ] T1.3 — Create `src/runner`; move runner logic
|
||||
- [ ] T1.4 — Create `src/scheduler`; move scheduler
|
||||
- [ ] T1.5 — Create `src/storage`; move store/paths
|
||||
- [ ] T1.6 — Create `src/platform/autostart`; move autostart logic
|
||||
- [ ] T1.7 — Create `src/platform/desktop`; move desktop integration
|
||||
- [ ] T1.8 — Delete empty `src/core`; build + test both platforms
|
||||
|
||||
### Phase 2 — Domain cleanup
|
||||
- [ ] T2.1 — Add `src/domain/schedule.go`; Schedule value object
|
||||
- [ ] T2.2 — Migrate `scheduler` to use Schedule
|
||||
- [ ] T2.3 — Split `domain.Job` (durable) from `domain.JobRuntime` (transient)
|
||||
- [ ] T2.4 — Update `storage`: load/save Job only; move runtime init
|
||||
|
||||
### Phase 3 — Application service layer
|
||||
- [ ] T3.1 — Create `src/app/service.go`; owns state behind mutex
|
||||
- [ ] T3.2 — Add `src/app/events.go`; Event types + Observer
|
||||
- [ ] T3.3 — Add state-mutating operations to service
|
||||
- [ ] T3.4 — Convert `scheduler` to use service; inject Clock
|
||||
- [ ] T3.5 — Move display helpers to `src/app/format.go`
|
||||
- [ ] T3.6 — Add `src/app` unit tests (no Fyne)
|
||||
|
||||
### Phase 4 — Carve up the GUI
|
||||
- [ ] T4.1 — Rename `gui` → `ui`; split app.go into run.go + mainwindow.go
|
||||
- [ ] T4.2 — Extract `jobs_view.go`
|
||||
- [ ] T4.3 — Extract `job_dialog.go`
|
||||
- [ ] T4.4 — Extract `history_view.go`
|
||||
- [ ] T4.5 — Extract `settings_view.go`
|
||||
- [ ] T4.6 — Extract `tray.go`, `singleinstance.go`, `layout.go`
|
||||
- [ ] T4.7 — Confirm app.go is gone; smoke test both platforms
|
||||
|
||||
### Phase 5 — Hardening & docs
|
||||
- [ ] T5.1 — Surface errors from service + storage
|
||||
- [ ] T5.2 — Introduce `autostart.Manager` interface
|
||||
- [ ] T5.3 — Fill test gaps (folder filtering, cleanup, migration, concurrency)
|
||||
- [ ] T5.4 — Run `go test -race ./...` clean on both platforms
|
||||
- [ ] T5.5 — Update docs (ARCHITECTURE.md, TESTS.md, README)
|
||||
|
||||
---
|
||||
|
||||
## 4. Definition of done
|
||||
|
||||
- `go vet ./...` clean; `go test -race ./...` green on Windows and Linux.
|
||||
- No package outside `ui` imports Fyne; no engine mutates UI state.
|
||||
- `domain.Job` has no `yaml:"-"` fields.
|
||||
- `app.Service` is the only writer of job/runtime state.
|
||||
- `src/ui` contains no file over ~250 lines; no single file over ~400.
|
||||
- `docs/ARCHITECTURE.md` matches the shipped structure.
|
||||
|
||||
## 5. Risks & mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|------|-----------|
|
||||
| Cross-platform code moves break the non-host OS build | Build with both `GOOS=windows` and `GOOS=linux` after each platform-touching task (T1.2, T1.3, T1.6, T1.7). |
|
||||
| Concurrency change (Phase 3/4) introduces subtle deadlocks | Keep the service mutex non-reentrant; never call back into the UI while holding it; cover with `-race` tests in T3.6. |
|
||||
| Behavior drift during moves | Characterization tests (T0.2) pin behavior before structural change. |
|
||||
| Large diff hard to review | Each task is a separate commit/PR; phases land independently. |
|
||||
+51
-37
@@ -2,6 +2,57 @@
|
||||
|
||||
All tests are located alongside source code in the `src/core/` package. Tests follow Go conventions with `*_test.go` filename patterns.
|
||||
|
||||
## Running Tests
|
||||
|
||||
### Using the test scripts
|
||||
|
||||
The repository provides convenience scripts to run all tests with static analysis:
|
||||
|
||||
**Unix/Linux/macOS:**
|
||||
```bash
|
||||
./scripts/test.sh
|
||||
```
|
||||
|
||||
**Windows:**
|
||||
```bash
|
||||
scripts\test.bat
|
||||
```
|
||||
|
||||
Both scripts run:
|
||||
1. `go vet ./...` — static analysis for common errors and suspicious code patterns
|
||||
2. `go test -race ./...` — tests with race condition detection enabled
|
||||
|
||||
### Manual test commands
|
||||
|
||||
Run all tests:
|
||||
```bash
|
||||
go test ./...
|
||||
```
|
||||
|
||||
Run all tests with race detection:
|
||||
```bash
|
||||
go test -race ./...
|
||||
```
|
||||
|
||||
Run tests with verbose output:
|
||||
```bash
|
||||
go test -v ./...
|
||||
```
|
||||
|
||||
Run a specific test by name:
|
||||
```bash
|
||||
go test -run TestRunJobWritesLogFile ./src/core
|
||||
```
|
||||
|
||||
Run tests with code coverage:
|
||||
```bash
|
||||
go test -cover ./src/core
|
||||
go test -coverprofile=coverage.out ./src/core
|
||||
go tool cover -html=coverage.out
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Test Files Overview
|
||||
|
||||
### store_test.go
|
||||
@@ -107,43 +158,6 @@ Tests Linux autostart entry creation via XDG Desktop Entry files.
|
||||
|
||||
---
|
||||
|
||||
## Running Tests
|
||||
|
||||
### Run all tests in the package
|
||||
```bash
|
||||
cd D:\Local\Git\gosentry
|
||||
go test ./src/core
|
||||
```
|
||||
|
||||
### Run tests with verbose output
|
||||
```bash
|
||||
go test -v ./src/core
|
||||
```
|
||||
|
||||
### Run specific test by name
|
||||
```bash
|
||||
go test -run TestRunJobWritesLogFile ./src/core
|
||||
```
|
||||
|
||||
### Run Windows-only tests (on Windows)
|
||||
```bash
|
||||
go test -v ./src/core # Windows build tags are active
|
||||
```
|
||||
|
||||
### Run Linux-only tests (on Linux)
|
||||
```bash
|
||||
go test -v ./src/core # Linux build tags are active
|
||||
```
|
||||
|
||||
### Run with code coverage
|
||||
```bash
|
||||
go test -cover ./src/core
|
||||
go test -coverprofile=coverage.out ./src/core
|
||||
go tool cover -html=coverage.out
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Test Design Principles
|
||||
|
||||
1. **Isolation** — Tests use `t.TempDir()` for file operations and `t.Setenv()` for environment variables to avoid affecting system state.
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
@echo off
|
||||
REM GoSentry test runner
|
||||
REM Runs go vet and go test with race detection
|
||||
|
||||
echo Running go vet...
|
||||
go vet ./...
|
||||
if errorlevel 1 (
|
||||
echo.
|
||||
echo ✗ go vet failed
|
||||
exit /b 1
|
||||
)
|
||||
|
||||
echo.
|
||||
echo Running go test with race detection...
|
||||
go test -race ./...
|
||||
if errorlevel 1 (
|
||||
echo.
|
||||
echo ✗ go test failed
|
||||
exit /b 1
|
||||
)
|
||||
|
||||
echo.
|
||||
echo ✓ All tests passed
|
||||
@@ -0,0 +1,16 @@
|
||||
#!/bin/bash
|
||||
|
||||
# GoSentry test runner
|
||||
# Runs go vet and go test with race detection
|
||||
|
||||
set -e
|
||||
|
||||
echo "Running go vet..."
|
||||
go vet ./...
|
||||
|
||||
echo ""
|
||||
echo "Running go test with race detection..."
|
||||
go test -race ./...
|
||||
|
||||
echo ""
|
||||
echo "✓ All tests passed"
|
||||
@@ -7,8 +7,142 @@ import (
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestRunJobLogFileAllHeaders(t *testing.T) {
|
||||
logsDir := t.TempDir()
|
||||
job := Job{
|
||||
ID: 99,
|
||||
Name: "Log Header Test",
|
||||
Command: echoCommand("header test output"),
|
||||
SuccessExitCodes: "0,1",
|
||||
}
|
||||
|
||||
record := RunJob(context.Background(), &job, "Schedule", logsDir)
|
||||
if record.LogFile == "" {
|
||||
t.Fatal("expected log file to be written")
|
||||
}
|
||||
|
||||
data, err := os.ReadFile(record.LogFile)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
content := string(data)
|
||||
|
||||
for _, want := range []string{
|
||||
"job_id: 99",
|
||||
"job_name: Log Header Test",
|
||||
"trigger: Schedule",
|
||||
"state: OK",
|
||||
"detail: ",
|
||||
"command: " + job.Command,
|
||||
"arguments: <empty>",
|
||||
"success_exit_codes: 0,1",
|
||||
"start_only: false",
|
||||
"stdout:",
|
||||
"stderr:",
|
||||
} {
|
||||
if !strings.Contains(content, want) {
|
||||
t.Errorf("log file missing %q:\n%s", want, content)
|
||||
}
|
||||
}
|
||||
|
||||
// The time header must use the documented format.
|
||||
for _, line := range strings.Split(content, "\n") {
|
||||
if strings.HasPrefix(line, "time: ") {
|
||||
ts := strings.TrimPrefix(line, "time: ")
|
||||
if _, err := time.Parse("2006-01-02 15:04:05", ts); err != nil {
|
||||
t.Errorf("time header %q does not match format 2006-01-02 15:04:05: %v", ts, err)
|
||||
}
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunJobRecordFields(t *testing.T) {
|
||||
job := Job{
|
||||
ID: 55,
|
||||
Name: "Record Fields Test",
|
||||
Command: echoCommand("record field check"),
|
||||
}
|
||||
|
||||
record := RunJob(context.Background(), &job, "Schedule", t.TempDir())
|
||||
|
||||
if record.JobID != job.ID {
|
||||
t.Errorf("JobID: got %d, want %d", record.JobID, job.ID)
|
||||
}
|
||||
if record.JobName != job.Name {
|
||||
t.Errorf("JobName: got %q, want %q", record.JobName, job.Name)
|
||||
}
|
||||
if record.Trigger != "Schedule" {
|
||||
t.Errorf("Trigger: got %q, want 'Schedule'", record.Trigger)
|
||||
}
|
||||
if record.State != "OK" {
|
||||
t.Errorf("State: got %q, want 'OK' (detail: %q)", record.State, record.Detail)
|
||||
}
|
||||
if record.LogFile == "" {
|
||||
t.Error("LogFile should be a non-empty path")
|
||||
}
|
||||
if _, err := time.Parse("2006-01-02 15:04:05", record.Time); err != nil {
|
||||
t.Errorf("Time format wrong, got %q: %v", record.Time, err)
|
||||
}
|
||||
if !strings.Contains(record.Output, "stdout:") {
|
||||
t.Errorf("Output missing 'stdout:', got:\n%s", record.Output)
|
||||
}
|
||||
if !strings.Contains(record.Output, "stderr:") {
|
||||
t.Errorf("Output missing 'stderr:', got:\n%s", record.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatOutput(t *testing.T) {
|
||||
got := formatOutput("hello world", "some error")
|
||||
want := "stdout:\nhello world\n\nstderr:\nsome error"
|
||||
if got != want {
|
||||
t.Errorf("formatOutput:\ngot: %q\nwant: %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatOutputEmptyStreams(t *testing.T) {
|
||||
got := formatOutput("", "")
|
||||
if !strings.Contains(got, "stdout:\n<empty>") {
|
||||
t.Errorf("empty stdout should show <empty>, got:\n%s", got)
|
||||
}
|
||||
if !strings.Contains(got, "stderr:\n<empty>") {
|
||||
t.Errorf("empty stderr should show <empty>, got:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLogArguments(t *testing.T) {
|
||||
cases := []struct{ input, want string }{
|
||||
{"", "<empty>"},
|
||||
{" ", "<empty>"},
|
||||
{"--flag", "--flag"},
|
||||
{"--flag\r\n--value", "--flag\n--value"},
|
||||
{"--flag\n--value", "--flag\n--value"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := logArguments(tc.input); got != tc.want {
|
||||
t.Errorf("logArguments(%q) = %q, want %q", tc.input, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeFileName(t *testing.T) {
|
||||
cases := []struct{ input, want string }{
|
||||
{"Hello Test", "Hello_Test"},
|
||||
{"job-1_ok", "job-1_ok"},
|
||||
{"!!!", "job"},
|
||||
{"", "job"},
|
||||
{"A/B:C", "A_B_C"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
if got := sanitizeFileName(tc.input); got != tc.want {
|
||||
t.Errorf("sanitizeFileName(%q) = %q, want %q", tc.input, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunJobWritesLogFile(t *testing.T) {
|
||||
logsDir := t.TempDir()
|
||||
job := Job{
|
||||
|
||||
@@ -6,6 +6,60 @@ import (
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestNextRunTimeRejectsInvalidSchedules(t *testing.T) {
|
||||
from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC)
|
||||
cases := []struct {
|
||||
schedule string
|
||||
desc string
|
||||
}{
|
||||
{"", "empty string"},
|
||||
{" ", "whitespace only"},
|
||||
{"@every", "bare @every without duration"},
|
||||
{"@every xyz", "invalid @every duration string"},
|
||||
{"@every -1s", "negative @every duration"},
|
||||
{"@every 0s", "zero @every duration"},
|
||||
{"not-a-cron", "invalid cron expression"},
|
||||
{"60 * * * *", "cron minute out of range"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
_, ok := nextRunTime(tc.schedule, from)
|
||||
if ok {
|
||||
t.Errorf("nextRunTime(%q) [%s]: expected false, got true", tc.schedule, tc.desc)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrepareNextRunSetsDisplayString(t *testing.T) {
|
||||
jobs := []Job{{Schedule: "*/5 * * * *", Enabled: true}}
|
||||
s := &Scheduler{jobs: &jobs}
|
||||
from := time.Date(2026, 6, 14, 12, 3, 0, 0, time.UTC)
|
||||
|
||||
s.prepareNextRun(&jobs[0], from)
|
||||
|
||||
want := "2026-06-14 12:05:00"
|
||||
if jobs[0].NextRun != want {
|
||||
t.Errorf("NextRun: got %q, want %q", jobs[0].NextRun, want)
|
||||
}
|
||||
wantDue := time.Date(2026, 6, 14, 12, 5, 0, 0, time.UTC)
|
||||
if !jobs[0].nextDue.Equal(wantDue) {
|
||||
t.Errorf("nextDue: got %v, want %v", jobs[0].nextDue, wantDue)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrepareNextRunSetsInvalidScheduleLabel(t *testing.T) {
|
||||
jobs := []Job{{Schedule: "not-a-cron", Enabled: true}}
|
||||
s := &Scheduler{jobs: &jobs}
|
||||
|
||||
s.prepareNextRun(&jobs[0], time.Now())
|
||||
|
||||
if jobs[0].NextRun != "Invalid schedule" {
|
||||
t.Errorf("NextRun: got %q, want 'Invalid schedule'", jobs[0].NextRun)
|
||||
}
|
||||
if !jobs[0].nextDue.IsZero() {
|
||||
t.Errorf("nextDue should be zero for invalid schedule, got %v", jobs[0].nextDue)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNextRunTimeSupportsEvery(t *testing.T) {
|
||||
from := time.Date(2026, 6, 14, 12, 0, 0, 0, time.UTC)
|
||||
next, ok := nextRunTime("@every 10s", from)
|
||||
|
||||
@@ -1,12 +1,178 @@
|
||||
package core
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"go.yaml.in/yaml/v4"
|
||||
)
|
||||
|
||||
func TestJobsRoundTrip(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "jobs.yaml")
|
||||
|
||||
original := []Job{
|
||||
{
|
||||
ID: 7,
|
||||
Name: "Backup data",
|
||||
Folder: "Maintenance",
|
||||
Schedule: "0 2 * * *",
|
||||
Command: "/usr/bin/backup",
|
||||
Arguments: "--compress\n--verbose",
|
||||
SuccessExitCodes: "0,1",
|
||||
StartOnly: true,
|
||||
Enabled: true,
|
||||
},
|
||||
}
|
||||
|
||||
if err := writeYAML(path, JobsFile{Jobs: original}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got, err := loadOrCreateJobs(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected 1 job, got %d", len(got))
|
||||
}
|
||||
|
||||
g, w := got[0], original[0]
|
||||
if g.ID != w.ID {
|
||||
t.Errorf("ID: got %d, want %d", g.ID, w.ID)
|
||||
}
|
||||
if g.Name != w.Name {
|
||||
t.Errorf("Name: got %q, want %q", g.Name, w.Name)
|
||||
}
|
||||
if g.Folder != w.Folder {
|
||||
t.Errorf("Folder: got %q, want %q", g.Folder, w.Folder)
|
||||
}
|
||||
if g.Schedule != w.Schedule {
|
||||
t.Errorf("Schedule: got %q, want %q", g.Schedule, w.Schedule)
|
||||
}
|
||||
if g.Command != w.Command {
|
||||
t.Errorf("Command: got %q, want %q", g.Command, w.Command)
|
||||
}
|
||||
if g.Arguments != w.Arguments {
|
||||
t.Errorf("Arguments: got %q, want %q", g.Arguments, w.Arguments)
|
||||
}
|
||||
if g.SuccessExitCodes != w.SuccessExitCodes {
|
||||
t.Errorf("SuccessExitCodes: got %q, want %q", g.SuccessExitCodes, w.SuccessExitCodes)
|
||||
}
|
||||
if g.StartOnly != w.StartOnly {
|
||||
t.Errorf("StartOnly: got %v, want %v", g.StartOnly, w.StartOnly)
|
||||
}
|
||||
if g.Enabled != w.Enabled {
|
||||
t.Errorf("Enabled: got %v, want %v", g.Enabled, w.Enabled)
|
||||
}
|
||||
|
||||
// Runtime fields must not survive the save→load round-trip.
|
||||
if g.LastRun != "" {
|
||||
t.Errorf("LastRun should be empty after load, got %q", g.LastRun)
|
||||
}
|
||||
if g.LastState != "" {
|
||||
t.Errorf("LastState should be empty after load, got %q", g.LastState)
|
||||
}
|
||||
if g.Logs != nil {
|
||||
t.Errorf("Logs should be nil after load, got %v", g.Logs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfigRoundTrip(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
paths := Paths{
|
||||
AppDir: dir,
|
||||
ConfigPath: filepath.Join(dir, ConfigFileName),
|
||||
}
|
||||
|
||||
want := Config{
|
||||
JobsDir: "/custom/jobs",
|
||||
LogsDir: "/custom/logs",
|
||||
MaxLogFiles: 50,
|
||||
MaxLogAgeDays: 14,
|
||||
StartOnLogin: true,
|
||||
KeepRunningInTray: false,
|
||||
NotifyOnFailure: false,
|
||||
}
|
||||
if err := writeYAML(paths.ConfigPath, want); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got, err := loadOrCreateConfig(paths)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if got.JobsDir != want.JobsDir {
|
||||
t.Errorf("JobsDir: got %q, want %q", got.JobsDir, want.JobsDir)
|
||||
}
|
||||
if got.LogsDir != want.LogsDir {
|
||||
t.Errorf("LogsDir: got %q, want %q", got.LogsDir, want.LogsDir)
|
||||
}
|
||||
if got.MaxLogFiles != want.MaxLogFiles {
|
||||
t.Errorf("MaxLogFiles: got %d, want %d", got.MaxLogFiles, want.MaxLogFiles)
|
||||
}
|
||||
if got.MaxLogAgeDays != want.MaxLogAgeDays {
|
||||
t.Errorf("MaxLogAgeDays: got %d, want %d", got.MaxLogAgeDays, want.MaxLogAgeDays)
|
||||
}
|
||||
if got.StartOnLogin != want.StartOnLogin {
|
||||
t.Errorf("StartOnLogin: got %v, want %v", got.StartOnLogin, want.StartOnLogin)
|
||||
}
|
||||
if got.KeepRunningInTray != want.KeepRunningInTray {
|
||||
t.Errorf("KeepRunningInTray: got %v, want %v", got.KeepRunningInTray, want.KeepRunningInTray)
|
||||
}
|
||||
if got.NotifyOnFailure != want.NotifyOnFailure {
|
||||
t.Errorf("NotifyOnFailure: got %v, want %v", got.NotifyOnFailure, want.NotifyOnFailure)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeJobsFillsDefaults(t *testing.T) {
|
||||
jobs := []Job{
|
||||
{Enabled: true},
|
||||
{Enabled: false},
|
||||
{ID: 5, Name: "Kept", Schedule: "*/10 * * * *", SuccessExitCodes: "0,1", Enabled: true},
|
||||
}
|
||||
|
||||
normalizeJobs(jobs)
|
||||
|
||||
// Blank enabled job gets default name, schedule, command, exit codes, and runtime state.
|
||||
if jobs[0].ID != 1 {
|
||||
t.Errorf("first auto ID: got %d, want 1", jobs[0].ID)
|
||||
}
|
||||
if jobs[0].Name != "Untitled job" {
|
||||
t.Errorf("default name: got %q, want 'Untitled job'", jobs[0].Name)
|
||||
}
|
||||
if jobs[0].Schedule != "@every 1m" {
|
||||
t.Errorf("default schedule: got %q, want '@every 1m'", jobs[0].Schedule)
|
||||
}
|
||||
if jobs[0].SuccessExitCodes != "0" {
|
||||
t.Errorf("default exit codes: got %q, want '0'", jobs[0].SuccessExitCodes)
|
||||
}
|
||||
if jobs[0].LastState != "Ready" {
|
||||
t.Errorf("enabled job state: got %q, want 'Ready'", jobs[0].LastState)
|
||||
}
|
||||
if jobs[0].NextRun != "After start" {
|
||||
t.Errorf("enabled job next run: got %q, want 'After start'", jobs[0].NextRun)
|
||||
}
|
||||
|
||||
// Disabled job is marked Paused.
|
||||
if jobs[1].LastState != "Paused" {
|
||||
t.Errorf("disabled job state: got %q, want 'Paused'", jobs[1].LastState)
|
||||
}
|
||||
if jobs[1].NextRun != "Paused" {
|
||||
t.Errorf("disabled job next run: got %q, want 'Paused'", jobs[1].NextRun)
|
||||
}
|
||||
|
||||
// Pre-set fields survive normalization unchanged.
|
||||
if jobs[2].ID != 5 {
|
||||
t.Errorf("pre-set ID should be preserved: got %d, want 5", jobs[2].ID)
|
||||
}
|
||||
if jobs[2].SuccessExitCodes != "0,1" {
|
||||
t.Errorf("pre-set exit codes should be preserved: got %q, want '0,1'", jobs[2].SuccessExitCodes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestJobsYAMLDoesNotPersistRuntimeNoise(t *testing.T) {
|
||||
jobs := []Job{
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user