Files
gosentry/src/storage/store_test.go
T
mix 18da021526 chore: land the remaining low-severity items from the whole-project review
Phase 11 of PROJECT_REVIEW_PLAN.md: the themed cleanup pass over every
low-severity finding still open (2.2-2.3, 3.4-3.6, 4.3-4.7, 6.4-6.7,
7.1-7.3, 8.2-8.3, 9.1-9.4, and the under-documented decisions in §10/§11).

Behavioral fixes:
- Reassign duplicate job IDs in a hand-edited jobs.json instead of letting
  two jobs share one runtime, schedule entry, and SeedStats bucket.
- Disambiguate run-log file names that collide within the same second.
- Compute AvgDurationMS as DurationSumMS/TimedRunCount instead of an
  incremental integer mean, so it always matches the seeded-from-logs
  average instead of drifting from truncation error.
- Clean absolute paths in ResolveConfiguredPath so two spellings of the
  same jobs file do not trigger a spurious adoption.
- Report InstallDesktopIcon failures through ErrorOccurred instead of
  discarding them silently.
- Move settingsView's blocking AutostartStatus (PowerShell on Windows) off
  the UI thread.
- Give notify-timing.tsv its own extension so CleanupLogs no longer
  manages it as a run log.
- Replace the settingsView Save handler's second copy of validateConfig's
  rules with a bare parse, letting the Service's own error surface.

Cleanups:
- Delete collectActivity, the dead yaml tags on RunRecord, and the
  logArguments/LogArguments alias.
- Fold the two systemTrayRegistered/mainWindowHidden globals into one
  trayState instance Run owns and threads through Settings and the
  single-instance reveal path.
- Fix stale comments/docs: the frozen window-size restore claim, a
  reference to a renamed recordRun, README's "Pause all" and notification
  wording, the PowerShell quoting note for TESTS.md's coverage command,
  and scripts/test.bat's UTF-8 checkmarks under a non-UTF-8 code page.
- Document the single-instance fallback's consequence and the
  unauthenticated instance-channel port in STANDARDS.md; record the
  config-shim retirement plan in ROADMAP.md.

3.5, 7.3, and 9.4 turned out to already be fixed by earlier phases; no
change needed for those three.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-07 03:35:10 +03:00

548 lines
17 KiB
Go

package storage
import (
"encoding/json"
"os"
"path/filepath"
"runtime"
"strings"
"testing"
"gitea.mixdep.ru/mix/gosentry/src/domain"
)
func TestJobsRoundTrip(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "jobs.json")
original := []domain.Job{
{
ID: 7,
Name: "Backup data",
Folder: "Maintenance",
Schedule: "0 2 * * *",
Command: "/usr/bin/backup",
Arguments: "--compress\n--verbose",
StartOnly: true,
Enabled: true,
},
}
if err := writeJSON(path, domain.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.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 state no longer lives on Job at all (it moved to domain.JobRuntime),
// so there is nothing transient that could survive the save→load round-trip.
}
func TestConfigRoundTrip(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
want := domain.Config{
JobsFile: "/custom/jobs/team.json",
LogsDir: "/custom/logs",
MaxLogFiles: 50,
MaxLogAgeDays: 14,
StartOnLogin: true,
KeepRunningInTray: true,
NotifyOnFailure: true,
}
if err := writeJSON(paths.ConfigPath, want); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.JobsFile != want.JobsFile {
t.Errorf("JobsFile: got %q, want %q", got.JobsFile, want.JobsFile)
}
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 := []domain.Job{
{Enabled: true},
{Enabled: false},
{ID: 5, Name: "Kept", Schedule: "*/10 * * * *", Enabled: true},
}
normalizeJobs(jobs)
// Blank enabled job gets default name, schedule, and command.
// normalizeJobs only fills durable configuration now; runtime status is built
// separately by domain.NewRuntime.
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)
}
// 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)
}
}
// TestNormalizeJobsReassignsDuplicateIDs pins the fix for a hand-edited
// jobs.json carrying two entries with the same ID: without reassignment both
// would share one JobRuntime, one schedule-cache entry, and one SeedStats
// bucket, so editing or deleting either would silently affect both.
func TestNormalizeJobsReassignsDuplicateIDs(t *testing.T) {
jobs := []domain.Job{
{ID: 5, Name: "First"},
{ID: 5, Name: "Second"},
{ID: 5, Name: "Third"},
}
normalizeJobs(jobs)
seen := make(map[int]bool, len(jobs))
for _, job := range jobs {
if seen[job.ID] {
t.Fatalf("ID %d assigned to more than one job after normalization: %+v", job.ID, jobs)
}
seen[job.ID] = true
}
if jobs[0].ID != 5 {
t.Errorf("first occurrence should keep its ID: got %d, want 5", jobs[0].ID)
}
if jobs[1].ID == 5 || jobs[2].ID == 5 {
t.Errorf("later duplicates should be reassigned away from 5: got %d, %d", jobs[1].ID, jobs[2].ID)
}
}
// TestResolveConfiguredPathCleansAbsolutePaths pins the fix for two spellings
// of the same absolute path (forward vs. backslashes) resolving to different
// strings: UpdateSettings compares this against Paths.JobsPath as strings to
// decide whether the jobs file is changing, so an uncleaned path here could
// trigger a spurious adoption against the file already in use.
func TestResolveConfiguredPathCleansAbsolutePaths(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("backslash vs. forward-slash spellings of the same path are a Windows-only ambiguity")
}
got := ResolveConfiguredPath(`C:\app`, "C:/data/jobs.json")
want := ResolveConfiguredPath(`C:\app`, `C:\data\jobs.json`)
if got != want {
t.Errorf("forward-slash and backslash spellings resolved differently: %q vs %q", got, want)
}
}
func TestLoadOrCreateConfigCreatesDefaultsOnFirstRun(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.JobsFile != "jobs.json" {
t.Errorf("default JobsFile = %q, want 'jobs.json'", got.JobsFile)
}
if got.LogsDir != "logs" {
t.Errorf("default LogsDir = %q, want 'logs'", got.LogsDir)
}
if got.MaxLogFiles != 100 {
t.Errorf("default MaxLogFiles = %d, want 100", got.MaxLogFiles)
}
if got.MaxLogAgeDays != 30 {
t.Errorf("default MaxLogAgeDays = %d, want 30", got.MaxLogAgeDays)
}
if got.DefaultTimeoutSeconds != 0 {
t.Errorf("default DefaultTimeoutSeconds = %d, want 0 (no timeout)", got.DefaultTimeoutSeconds)
}
if got.Theme != domain.ThemeGoSentry {
t.Errorf("default Theme = %q, want %q", got.Theme, domain.ThemeGoSentry)
}
if got.JobListView != domain.JobListViewDetailed {
t.Errorf("default JobListView = %q, want %q", got.JobListView, domain.JobListViewDetailed)
}
// The function must have written the defaults to gosentry.json.
if _, err := os.Stat(paths.ConfigPath); err != nil {
t.Errorf("gosentry.json should have been created: %v", err)
}
}
// TestLoadOrCreateConfigPreservesZeroRetentionLimits verifies that 0 in
// max_log_files / max_log_age_days is read back as 0 ("keep everything"), not
// backfilled to the 100 / 30 defaults, since a config that already has the
// field set is not the "field is missing" case loadOrCreateConfig backfills.
func TestLoadOrCreateConfigPreservesZeroRetentionLimits(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
want := domain.DefaultConfig()
want.MaxLogFiles = 0
want.MaxLogAgeDays = 0
if err := writeJSON(paths.ConfigPath, want); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.MaxLogFiles != 0 {
t.Errorf("MaxLogFiles: got %d, want 0 (unlimited)", got.MaxLogFiles)
}
if got.MaxLogAgeDays != 0 {
t.Errorf("MaxLogAgeDays: got %d, want 0 (unlimited)", got.MaxLogAgeDays)
}
}
// TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun verifies that a missing
// jobs.json is created with the sample jobs from defaultJobs, so a new user
// sees scheduled and manual execution without inventing a command.
func TestLoadOrCreateJobsSeedsSampleJobsOnFirstRun(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "jobs.json")
got, err := loadOrCreateJobs(path)
if err != nil {
t.Fatal(err)
}
want := defaultJobs()
if len(got) != len(want) {
t.Fatalf("got %d jobs, want %d", len(got), len(want))
}
for i := range want {
if got[i].Name != want[i].Name || got[i].Schedule != want[i].Schedule || got[i].Command != want[i].Command || got[i].Enabled != want[i].Enabled {
t.Errorf("job %d = %+v, want %+v", i, got[i], want[i])
}
}
// The function must have written the seeded jobs to jobs.json.
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("jobs.json should have been created: %v", err)
}
var file domain.JobsFile
if err := json.Unmarshal(data, &file); err != nil {
t.Fatal(err)
}
if len(file.Jobs) != len(want) {
t.Errorf("jobs.json has %d jobs, want %d", len(file.Jobs), len(want))
}
}
// TestLoadOrCreateConfigMigratesLegacyThemeDefault covers a gosentry.json that
// still stores the retired "default" theme value: load normalizes it to system.
func TestLoadOrCreateConfigMigratesLegacyThemeDefault(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
legacy := map[string]any{
"jobs_file": "jobs.json",
"logs_dir": "logs",
"max_log_files": 100,
"max_log_age_days": 30,
"theme": "default",
}
if err := writeJSON(paths.ConfigPath, legacy); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if got.Theme != domain.ThemeSystem {
t.Errorf("migrated Theme = %q, want %q", got.Theme, domain.ThemeSystem)
}
}
// TestLoadOrCreateConfigKeepsZeroTimeoutOnReload guards the "0 = no timeout"
// setting against being normalized away when an existing gosentry.json is read
// back. Loading must not treat 0 as a missing value.
func TestLoadOrCreateConfigKeepsZeroTimeoutOnReload(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
// First call writes the defaults (DefaultTimeoutSeconds = 0) to disk.
if _, err := loadOrCreateConfig(paths); err != nil {
t.Fatal(err)
}
// Second call takes the "file exists" branch, where normalization runs.
reloaded, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
if reloaded.DefaultTimeoutSeconds != 0 {
t.Errorf("reloaded DefaultTimeoutSeconds = %d, want 0 (no timeout)", reloaded.DefaultTimeoutSeconds)
}
}
// TestLoadOrCreateConfigMigratesJobsDir covers a gosentry.json written before
// the setting named a file: the old jobs_dir keeps pointing at the same jobs
// file, and the retired key is dropped so it is not written back.
func TestLoadOrCreateConfigMigratesJobsDir(t *testing.T) {
dir := t.TempDir()
paths := Paths{
AppDir: dir,
ConfigPath: filepath.Join(dir, ConfigFileName),
}
legacy := map[string]any{
"jobs_dir": filepath.Join(dir, "shared"),
"logs_dir": "logs",
"max_log_files": 100,
"max_log_age_days": 30,
}
if err := writeJSON(paths.ConfigPath, legacy); err != nil {
t.Fatal(err)
}
got, err := loadOrCreateConfig(paths)
if err != nil {
t.Fatal(err)
}
want := filepath.Join(dir, "shared", JobsFileName)
if got.JobsFile != want {
t.Errorf("migrated JobsFile: got %q, want %q", got.JobsFile, want)
}
if got.JobsDir != "" {
t.Errorf("legacy JobsDir should be cleared, got %q", got.JobsDir)
}
// The migrated config must not carry the retired key once it is saved.
store := &Store{Paths: paths, Config: got}
if err := store.SaveConfig(); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(paths.ConfigPath)
if err != nil {
t.Fatal(err)
}
if strings.Contains(string(data), "jobs_dir") {
t.Errorf("saved config should not contain jobs_dir:\n%s", data)
}
}
// TestLoadJobsFileReportsMissingWithoutCreating covers the loader the Settings
// tab uses to decide between adopting a jobs file and writing the current jobs
// to it: a missing file is reported as "not found" rather than an error, and —
// unlike the startup path — is not seeded with sample jobs.
func TestLoadJobsFileReportsMissingWithoutCreating(t *testing.T) {
dir := t.TempDir()
missing := filepath.Join(dir, "nothing-here.json")
jobs, found, err := LoadJobsFile(missing)
if err != nil {
t.Fatalf("missing file should not be an error: %v", err)
}
if found || jobs != nil {
t.Errorf("missing file: got found=%v jobs=%+v, want false/nil", found, jobs)
}
if _, err := os.Stat(missing); !os.IsNotExist(err) {
t.Error("LoadJobsFile must not create the file it was asked about")
}
// An existing file comes back normalized, so a hand-written jobs file gains
// its IDs and defaults before the application adopts it.
path := filepath.Join(dir, "hand-written.json")
if err := writeJSON(path, domain.JobsFile{Jobs: []domain.Job{{Name: "No ID"}}}); err != nil {
t.Fatal(err)
}
jobs, found, err = LoadJobsFile(path)
if err != nil {
t.Fatal(err)
}
if !found || len(jobs) != 1 {
t.Fatalf("existing file: got found=%v jobs=%+v, want true and one job", found, jobs)
}
if jobs[0].ID != 1 || jobs[0].Schedule == "" || jobs[0].Command == "" {
t.Errorf("loaded job should be normalized, got %+v", jobs[0])
}
}
// TestApplyConfigPathsDerivesJobsDir checks that the jobs file drives both
// resolved paths: relative values resolve against the program folder, and the
// containing directory comes from the file name the user chose.
func TestApplyConfigPathsDerivesJobsDir(t *testing.T) {
dir := t.TempDir()
store := &Store{
Paths: Paths{AppDir: dir},
Config: domain.Config{JobsFile: filepath.Join("shared", "team.json"), LogsDir: "logs"},
}
store.applyConfigPaths()
if want := filepath.Join(dir, "shared", "team.json"); store.Paths.JobsPath != want {
t.Errorf("JobsPath: got %q, want %q", store.Paths.JobsPath, want)
}
if want := filepath.Join(dir, "shared"); store.Paths.JobsDir != want {
t.Errorf("JobsDir: got %q, want %q", store.Paths.JobsDir, want)
}
}
// TestJobTimeoutRoundTripsThreeStates pins the on-disk encoding that keeps
// "inherit" and "no timeout" distinguishable: nil is omitted entirely, while an
// explicit 0 is written and read back as a set value.
func TestJobTimeoutRoundTripsThreeStates(t *testing.T) {
jobs := []domain.Job{
{ID: 1, Name: "Inherit", TimeoutSeconds: nil},
{ID: 2, Name: "No timeout", TimeoutSeconds: domain.TimeoutSecondsPtr(0)},
{ID: 3, Name: "Own", TimeoutSeconds: domain.TimeoutSecondsPtr(45)},
}
data, err := json.Marshal(domain.JobsFile{Jobs: jobs})
if err != nil {
t.Fatal(err)
}
if want := `"timeout_seconds":0`; !strings.Contains(string(data), want) {
t.Fatalf("explicit zero timeout should be written as %s:\n%s", want, data)
}
var got domain.JobsFile
if err := json.Unmarshal(data, &got); err != nil {
t.Fatal(err)
}
if got.Jobs[0].TimeoutSeconds != nil {
t.Errorf("unset timeout should stay nil, got %d", *got.Jobs[0].TimeoutSeconds)
}
if got.Jobs[1].TimeoutSeconds == nil || *got.Jobs[1].TimeoutSeconds != 0 {
t.Errorf("explicit zero timeout should survive the round trip, got %v", got.Jobs[1].TimeoutSeconds)
}
if got.Jobs[2].TimeoutSeconds == nil || *got.Jobs[2].TimeoutSeconds != 45 {
t.Errorf("per-job timeout should survive the round trip, got %v", got.Jobs[2].TimeoutSeconds)
}
}
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 json tag.
jobs := []domain.Job{
{
ID: 1,
Name: "Clean job",
Schedule: "@every 10s",
Command: echoCommand("ok"),
Enabled: true,
},
}
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 json should not contain %q:\n%s", unwanted, text)
}
}
}
// TestWriteJSONReplacesFileAtomically pins the durability fix: writeJSON must
// never truncate the destination in place. It writes through a temp file and
// renames over the target, so a reader can never observe a partially written
// file, and an existing file survives untouched if the marshal fails first.
func TestWriteJSONReplacesFileAtomically(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "gosentry.json")
original := domain.DefaultConfig()
original.LogsDir = "logs-original"
if err := writeJSON(path, original); err != nil {
t.Fatal(err)
}
updated := domain.DefaultConfig()
updated.LogsDir = "logs-updated"
if err := writeJSON(path, updated); err != nil {
t.Fatal(err)
}
data, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
var got domain.Config
if err := json.Unmarshal(data, &got); err != nil {
t.Fatal(err)
}
if got.LogsDir != "logs-updated" {
t.Fatalf("LogsDir = %q, want %q", got.LogsDir, "logs-updated")
}
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
if len(entries) != 1 {
t.Fatalf("expected only the final file in %s, got %v", dir, entries)
}
}