From 9af279e28f1fab0cd1fe7360928b245573175f28 Mon Sep 17 00:00:00 2001 From: mixeme Date: Mon, 22 Jun 2026 21:57:39 +0300 Subject: [PATCH] P1.6: update tests and docs for JSON storage + exit-code removal - storage/store_test.go: switch TestJobsYAMLDoesNotPersistRuntimeNoise to json.Marshal (renamed to TestJobsJSONDoesNotPersistRuntimeNoise); add TestLoadOrCreateJobsMigratesFromLegacy to cover the jobs YAML import path - app/operations_test.go: update newTempService paths to .json filenames - docs/TESTS.md: reflect JSON round-trip, drop SuccessExitCodes tests, remove Exit Code Flexibility design-principle entry - docs/PRE-RELEASE-TASKS.md: mark P1.6 complete Co-Authored-By: Claude Sonnet 4.6 --- docs/PRE-RELEASE-TASKS.md | 2 +- docs/TESTS.md | 28 ++++++++++-------------- src/app/operations_test.go | 4 ++-- src/storage/store_test.go | 45 ++++++++++++++++++++++++++++++++++---- 4 files changed, 56 insertions(+), 23 deletions(-) diff --git a/docs/PRE-RELEASE-TASKS.md b/docs/PRE-RELEASE-TASKS.md index f986a2e..e11be2f 100644 --- a/docs/PRE-RELEASE-TASKS.md +++ b/docs/PRE-RELEASE-TASKS.md @@ -85,7 +85,7 @@ These land together because both edit `domain/job.go` and `storage/store.go`. - [x] P1.3 — `gosentry.json` / `jobs.json` paths; drop pysentry name - [x] P1.4 — One-time YAML import - [x] P1.5 — Remove `SuccessExitCodes` across code -- [ ] P1.6 — Update storage/runner/format tests + TESTS.md +- [x] P1.6 — Update storage/runner/format tests + TESTS.md ### Phase 2 — PySentry legacy removal - [ ] P2.1 — Windows autostart legacy code diff --git a/docs/TESTS.md b/docs/TESTS.md index ab66815..7dab93b 100644 --- a/docs/TESTS.md +++ b/docs/TESTS.md @@ -98,7 +98,7 @@ Tests all mutating operations on the Service, scheduler integration, and setting | Test | Purpose | |------|---------| -| `TestCreateJobAssignsIDAndEmits` | Verifies that `CreateJob` assigns a unique ID, persists to YAML, and emits `JobChanged`. | +| `TestCreateJobAssignsIDAndEmits` | Verifies that `CreateJob` assigns a unique ID, persists to JSON, and emits `JobChanged`. | | `TestCreateJobValidates` | Verifies that `CreateJob` rejects jobs with an invalid schedule. | | `TestUpdateJobKeepsRuntimeAndReflectsDisable` | Verifies that `UpdateJob` preserves existing runtime state and disables a job correctly. | | `TestUpdateJobReenablesPausedJob` | Verifies that re-enabling a previously-disabled job clears the paused runtime state. | @@ -160,9 +160,8 @@ Tests display-formatting helpers used by the UI. | `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. | | `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". | | `TestDisplayArguments` | Verifies that an empty arguments string shows "None". | -| `TestDisplaySuccessExitCodes` | Verifies that an empty exit-codes string shows the default "0". | | `TestDisplayRunMode` | Verifies run-mode labels for normal and start-only modes. | -| `TestDisplayInvocation` | Verifies that the full invocation display string includes command, arguments, and exit codes. | +| `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). | --- @@ -171,16 +170,17 @@ Tests display-formatting helpers used by the UI. **Package:** `storage` -Tests YAML round-tripping, migration, and default generation. +Tests JSON round-tripping, YAML migration import, and default generation. | Test | Purpose | |------|---------| -| `TestJobsRoundTrip` | Verifies that jobs saved to YAML are reloaded with identical field values. | -| `TestConfigRoundTrip` | Verifies that settings saved to YAML are reloaded with identical field values. | -| `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default enabled state for jobs missing those fields. | -| `TestLoadOrCreateConfigMigratesFromLegacy` | Verifies that the old flat-config format is migrated to the current `gosentry.yaml` + `jobs_dir` layout on first load. | +| `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. | +| `TestConfigRoundTrip` | Verifies that settings saved to JSON are reloaded with identical field values. | +| `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command for jobs missing those fields. | +| `TestLoadOrCreateConfigMigratesFromLegacy` | Verifies that when `gosentry.json` is absent but `gosentry.yaml` exists the config is imported from the legacy YAML file on first load. | +| `TestLoadOrCreateJobsMigratesFromLegacy` | Verifies that when `jobs.json` is absent but `jobs.yaml` exists the jobs are imported from the legacy YAML file on first load. | | `TestLoadOrCreateConfigCreatesDefaultsOnFirstRun` | Verifies that a missing config file is created with sane defaults and a sample job. | -| `TestJobsYAMLDoesNotPersistRuntimeNoise` | Verifies that `jobs.yaml` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. | +| `TestJobsJSONDoesNotPersistRuntimeNoise` | Verifies that `jobs.json` does not persist runtime state (LastRun, NextRun, etc.). Only durable job fields are stored. | --- @@ -232,8 +232,7 @@ Tests command execution, exit code handling, output capture, and Windows-specifi | Test | Purpose | |------|---------| -| `TestRunJobAcceptsConfiguredExitCode` | Verifies that exit codes in `SuccessExitCodes` result in "OK" status even when nonzero. | -| `TestRunJobRejectsUnconfiguredExitCode` | Verifies that exit codes absent from `SuccessExitCodes` result in "Failed" status. | +| `TestRunJobFailsOnNonZeroExitCode` | Verifies that a nonzero process exit code results in "Failed" status with an "exit code N" detail. | #### Start-only mode @@ -246,7 +245,6 @@ Tests command execution, exit code handling, output capture, and Windows-specifi | Test | Platform | Purpose | |------|----------|---------| -| `TestParseExitCodes` | All | Verifies that mixed-separator exit-code strings (comma, semicolon, newline) are parsed correctly. | | `TestDirectCommandDoesNotHideWindow` | Windows | Verifies that direct executable commands do not request hidden-window startup. | | `TestShellCommandHidesWindow` | Windows | Verifies that shell commands request hidden-window startup to prevent console flash. | | `TestShellCommandUsesWindowsSafeQuoting` | Windows | Verifies `cmd.exe /S /C` quoting for paths with spaces and special characters. | @@ -334,11 +332,9 @@ Tests pure helper functions in the jobs view (no Fyne widget construction). 4. **Event-driven correctness** — `app` tests subscribe to the event bus and assert that the expected events are emitted, rather than inspecting internal fields directly. -5. **Exit Code Flexibility** — The `SuccessExitCodes` field allows jobs to treat nonzero exit codes as success, tested explicitly. +5. **Path Handling** — Extensive tests cover Windows path quoting, spaces in paths, and case-insensitive matching to avoid subtle shell escaping bugs. -6. **Path Handling** — Extensive tests cover Windows path quoting, spaces in paths, and case-insensitive matching to avoid subtle shell escaping bugs. - -7. **Start-Only Mode** — Special handling for long-running processes that should be launched but not waited on, tested separately from normal execution flow. +6. **Start-Only Mode** — Special handling for long-running processes that should be launched but not waited on, tested separately from normal execution flow. --- diff --git a/src/app/operations_test.go b/src/app/operations_test.go index e67e178..755b34a 100644 --- a/src/app/operations_test.go +++ b/src/app/operations_test.go @@ -20,9 +20,9 @@ func newTempService(t *testing.T, jobs []domain.Job) *Service { Paths: storage.Paths{ ExecutablePath: filepath.Join(dir, "gosentry"), AppDir: dir, - ConfigPath: filepath.Join(dir, "gosentry.yaml"), + ConfigPath: filepath.Join(dir, "gosentry.json"), JobsDir: dir, - JobsPath: filepath.Join(dir, "jobs.yaml"), + JobsPath: filepath.Join(dir, "jobs.json"), LogsDir: filepath.Join(dir, "logs"), }, Config: domain.Config{JobsDir: ".", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30}, diff --git a/src/storage/store_test.go b/src/storage/store_test.go index 635a2a9..d055669 100644 --- a/src/storage/store_test.go +++ b/src/storage/store_test.go @@ -1,6 +1,7 @@ package storage import ( + "encoding/json" "os" "path/filepath" "strings" @@ -226,10 +227,10 @@ func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) { } } -func TestJobsYAMLDoesNotPersistRuntimeNoise(t *testing.T) { +func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) { // Job carries only durable configuration; runtime state lives in // domain.JobRuntime and is never marshalled. This guards against a future - // runtime field accidentally being added back onto Job with a yaml tag. + // runtime field accidentally being added back onto Job with a json tag. jobs := []domain.Job{ { ID: 1, @@ -240,14 +241,50 @@ func TestJobsYAMLDoesNotPersistRuntimeNoise(t *testing.T) { }, } - data, err := yaml.Marshal(domain.JobsFile{Jobs: jobs}) + data, err := json.Marshal(domain.JobsFile{Jobs: jobs}) if err != nil { t.Fatal(err) } text := string(data) for _, unwanted := range []string{"last_run", "next_run", "last_state", "activity", "last_output", "stdout"} { if strings.Contains(text, unwanted) { - t.Fatalf("jobs yaml should not contain %q:\n%s", unwanted, text) + t.Fatalf("jobs json should not contain %q:\n%s", unwanted, text) } } } + +// TestLoadOrCreateJobsMigratesFromLegacy verifies that when jobs.json is absent +// but jobs.yaml exists the jobs are read from the legacy YAML file. +func TestLoadOrCreateJobsMigratesFromLegacy(t *testing.T) { + dir := t.TempDir() + jsonPath := filepath.Join(dir, JobsFileName) // jobs.json — not created + + legacy := yamlJobsFile{ + Jobs: []yamlJob{ + {ID: 10, Name: "Legacy job", Schedule: "@every 5m", Command: "echo legacy", Enabled: true}, + }, + } + if err := writeYAML(filepath.Join(dir, legacyYAMLJobsFileName), legacy); err != nil { + t.Fatal(err) + } + + got, err := loadOrCreateJobs(jsonPath) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("expected 1 job, got %d", len(got)) + } + if got[0].ID != 10 { + t.Errorf("ID: got %d, want 10", got[0].ID) + } + if got[0].Name != "Legacy job" { + t.Errorf("Name: got %q, want 'Legacy job'", got[0].Name) + } + if got[0].Schedule != "@every 5m" { + t.Errorf("Schedule: got %q, want '@every 5m'", got[0].Schedule) + } + if !got[0].Enabled { + t.Errorf("Enabled: got false, want true") + } +}