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 <noreply@anthropic.com>
This commit is contained in:
mixeme
2026-06-22 21:57:39 +03:00
parent fb149899e2
commit 9af279e28f
4 changed files with 56 additions and 23 deletions
+1 -1
View File
@@ -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.3 — `gosentry.json` / `jobs.json` paths; drop pysentry name
- [x] P1.4 — One-time YAML import - [x] P1.4 — One-time YAML import
- [x] P1.5 — Remove `SuccessExitCodes` across code - [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 ### Phase 2 — PySentry legacy removal
- [ ] P2.1 — Windows autostart legacy code - [ ] P2.1 — Windows autostart legacy code
+12 -16
View File
@@ -98,7 +98,7 @@ Tests all mutating operations on the Service, scheduler integration, and setting
| Test | Purpose | | 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. | | `TestCreateJobValidates` | Verifies that `CreateJob` rejects jobs with an invalid schedule. |
| `TestUpdateJobKeepsRuntimeAndReflectsDisable` | Verifies that `UpdateJob` preserves existing runtime state and disables a job correctly. | | `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. | | `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. | | `TestEventText` | Verifies trigger-type labels for scheduled, manual, and UI triggers. |
| `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". | | `TestDisplayFolder` | Verifies that an empty folder string shows "No folder". |
| `TestDisplayArguments` | Verifies that an empty arguments string shows "None". | | `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. | | `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). | | `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` **Package:** `storage`
Tests YAML round-tripping, migration, and default generation. Tests JSON round-tripping, YAML migration import, and default generation.
| Test | Purpose | | Test | Purpose |
|------|---------| |------|---------|
| `TestJobsRoundTrip` | Verifies that jobs saved to YAML are reloaded with identical field values. | | `TestJobsRoundTrip` | Verifies that jobs saved to JSON are reloaded with identical field values. |
| `TestConfigRoundTrip` | Verifies that settings saved to YAML 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 enabled state for jobs missing those fields. | | `TestNormalizeJobsFillsDefaults` | Verifies that `normalizeJobs` assigns sequential IDs and sets default name, schedule, and command 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. | | `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. | | `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 | | Test | Purpose |
|------|---------| |------|---------|
| `TestRunJobAcceptsConfiguredExitCode` | Verifies that exit codes in `SuccessExitCodes` result in "OK" status even when nonzero. | | `TestRunJobFailsOnNonZeroExitCode` | Verifies that a nonzero process exit code results in "Failed" status with an "exit code N" detail. |
| `TestRunJobRejectsUnconfiguredExitCode` | Verifies that exit codes absent from `SuccessExitCodes` result in "Failed" status. |
#### Start-only mode #### Start-only mode
@@ -246,7 +245,6 @@ Tests command execution, exit code handling, output capture, and Windows-specifi
| Test | Platform | Purpose | | 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. | | `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. | | `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. | | `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. 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. 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. **Start-Only Mode** — Special handling for long-running processes that should be launched but not waited on, tested separately from normal execution flow.
--- ---
+2 -2
View File
@@ -20,9 +20,9 @@ func newTempService(t *testing.T, jobs []domain.Job) *Service {
Paths: storage.Paths{ Paths: storage.Paths{
ExecutablePath: filepath.Join(dir, "gosentry"), ExecutablePath: filepath.Join(dir, "gosentry"),
AppDir: dir, AppDir: dir,
ConfigPath: filepath.Join(dir, "gosentry.yaml"), ConfigPath: filepath.Join(dir, "gosentry.json"),
JobsDir: dir, JobsDir: dir,
JobsPath: filepath.Join(dir, "jobs.yaml"), JobsPath: filepath.Join(dir, "jobs.json"),
LogsDir: filepath.Join(dir, "logs"), LogsDir: filepath.Join(dir, "logs"),
}, },
Config: domain.Config{JobsDir: ".", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30}, Config: domain.Config{JobsDir: ".", LogsDir: "logs", MaxLogFiles: 100, MaxLogAgeDays: 30},
+41 -4
View File
@@ -1,6 +1,7 @@
package storage package storage
import ( import (
"encoding/json"
"os" "os"
"path/filepath" "path/filepath"
"strings" "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 // Job carries only durable configuration; runtime state lives in
// domain.JobRuntime and is never marshalled. This guards against a future // 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{ jobs := []domain.Job{
{ {
ID: 1, 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
text := string(data) text := string(data)
for _, unwanted := range []string{"last_run", "next_run", "last_state", "activity", "last_output", "stdout"} { for _, unwanted := range []string{"last_run", "next_run", "last_state", "activity", "last_output", "stdout"} {
if strings.Contains(text, unwanted) { 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")
}
}