diff --git a/cmd/server/tests/extractor_version_test.go b/cmd/server/tests/extractor_version_test.go index 61864615..31084783 100644 --- a/cmd/server/tests/extractor_version_test.go +++ b/cmd/server/tests/extractor_version_test.go @@ -17,11 +17,9 @@ package main // equal → stable (second corruption persists) import ( - "bytes" "context" "encoding/json" "fmt" - "net/http" "os" "path/filepath" "strconv" @@ -58,14 +56,11 @@ func corruptItemColumn(t *testing.T, setup *TestServerSetup, itemID pgtype.UUID, func setExtractorVersion(t *testing.T, setup *TestServerSetup, v int) { t.Helper() - body, _ := json.Marshal(map[string]string{"key": "scan_extractor_version", "value": strconv.Itoa(v)}) - req, _ := http.NewRequest("PUT", setup.Server.URL+"/api/system/settings", bytes.NewBuffer(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+setup.Token) - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - defer func() { _ = resp.Body.Close() }() - require.Equal(t, http.StatusOK, resp.StatusCode, "settings API is the only sanctioned write path") + // Restore-to-prior (not a hardcoded value): the shared dev DB's + // setting must survive the suite exactly as the test found it — if a + // parser-fix bump was in flight, a hardcoded 1 would silently cancel + // it and the live server would stop re-extracting. + setSystemSettingRestored(t, setup, "scan_extractor_version", strconv.Itoa(v)) } func TestExtractorVersionReextract(t *testing.T) { @@ -123,7 +118,8 @@ func TestExtractorVersionReextract(t *testing.T) { // re-extract once: the corruption heals and the fresh manifest // carries ev=2. setExtractorVersion(t, setup, 2) - t.Cleanup(func() { setExtractorVersion(t, setup, 1) }) + // Prior-value restore rides inside setExtractorVersion — never a + // hardcoded 1, so a deliberate dev bump survives the suite. scanTestFolder(t, setup, admin, folder) item, found = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Probe Author/EV Probe Book/EV Probe Book.epub")) require.True(t, found) @@ -168,7 +164,8 @@ func TestExtractorVersionMultifile(t *testing.T) { // Bump → the ev gate fires through multifileSidecarChanged → the // editorial apply re-derives the title from the folder basename. setExtractorVersion(t, setup, 2) - t.Cleanup(func() { setExtractorVersion(t, setup, 1) }) + // Prior-value restore rides inside setExtractorVersion — never a + // hardcoded 1, so a deliberate dev bump survives the suite. scanTestFolder(t, setup, admin, folder) book, found = findAudioItemByPath(t, setup, libID, filepath.ToSlash("EV Audio Author/EV Audio Title")) require.True(t, found) diff --git a/cmd/server/tests/phase13_test.go b/cmd/server/tests/phase13_test.go index 6ddce7d5..a6bee305 100644 --- a/cmd/server/tests/phase13_test.go +++ b/cmd/server/tests/phase13_test.go @@ -32,11 +32,11 @@ import ( // the default 1 for the test's duration. Prior interrupted runs can // leave it bumped (an early test failure aborts before its restore // cleanup registers), and every stamp assertion in these tests assumes -// the default. The cleanup restores 1 — the repo's steady state. +// the default. Restore-to-prior rides inside setExtractorVersion: the +// suite leaves the setting exactly as it found it. func pinExtractorVersionOne(t *testing.T, setup *TestServerSetup) { t.Helper() setExtractorVersion(t, setup, 1) - t.Cleanup(func() { setExtractorVersion(t, setup, 1) }) } // forceScanTestFolder runs a FORCE scan over one folder. diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index ca84928a..7c1ea82a 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -25,6 +25,7 @@ import ( "github.com/go-playground/validator/v10" "github.com/google/uuid" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" "github.com/labstack/echo/v5" @@ -181,6 +182,54 @@ func getUploadPath() string { return "./uploads" // Host path (left side of volume mount) } +// setSystemSettingRestored writes a system setting through the sanctioned +// API path and registers a cleanup that restores the value the row had +// BEFORE the write. The testuser-experience rule: the suite must leave +// shared settings exactly as it found them — a hardcoded restore would +// silently clobber a deliberate dev bump (e.g. scan_extractor_version +// pinned mid-deploy), and the live dev server obeys this setting on its +// next poll, so what tests write here reaches real scans. +// +// Writes ride the settings API (never raw SQL) because the registry +// caches values and only the API path reloads it. If the row did not +// exist before the write, the test value is left in place and the test +// log says so — there is no sanctioned delete; settings rows that +// matter (scan_extractor_version) always exist. +func setSystemSettingRestored(t *testing.T, setup *TestServerSetup, key, value string) { + t.Helper() + var prior *string + err := setup.DBPool.QueryRow(context.Background(), + `SELECT setting_value FROM system_settings WHERE setting_key = $1`, key).Scan(&prior) + if err != nil && err != pgx.ErrNoRows { + require.NoError(t, err, "failed to read prior value of %s", key) + } + + setSettingViaAPI(t, setup, key, value) + + if prior == nil { + t.Logf("setSystemSettingRestored: %s had no prior row; leaving %q in place after the test", key, value) + return + } + priorValue := *prior + t.Cleanup(func() { + setSettingViaAPI(t, setup, key, priorValue) + }) +} + +// setSettingViaAPI PUTs one setting through /api/system/settings — the +// only sanctioned write path (the settings registry reloads here). +func setSettingViaAPI(t *testing.T, setup *TestServerSetup, key, value string) { + t.Helper() + body, _ := json.Marshal(map[string]string{"key": key, "value": value}) + req, _ := http.NewRequest("PUT", setup.Server.URL+"/api/system/settings", bytes.NewBuffer(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+setup.Token) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer func() { _ = resp.Body.Close() }() + require.Equal(t, http.StatusOK, resp.StatusCode, "settings API is the only sanctioned write path") +} + // getCachePath returns the appropriate cache path based on runtime environment func getCachePath() string { // Check for explicit override first