refactor(snapshot): address review follow-ups
- commit
- 91368cbc8cd9f94173a2b4704d83376724321067
- author
- brice · · 2026-07-22 05:34 UTC
- parent
- 5f10d2e
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +122 -92
M internal/app/offline_kill_retry_test.go +46 -82
| @@ -39,86 +39,50 @@ func (f *offlineSnapshotFake) DeleteTombstone(context.Context, string) error { | |||
| 39 | 39 | return f.deleteTombstoneErr | |
| 40 | 40 | } | |
| 41 | 41 | ||
| 42 | - | func TestRunKillOfflineRetainsTombstoneAndMetadataUntilBothSourcesAreDeleted(t *testing.T) { | |
| 43 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | |
| 44 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | |
| 45 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | |
| 46 | - | ||
| 47 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | |
| 48 | - | require.NoError(t, err) | |
| 49 | - | now := time.Now().UnixNano() | |
| 50 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | |
| 51 | - | require.NoError(t, p.Close()) | |
| 52 | - | ||
| 53 | - | injected := errors.New("legacy unlink failed") | |
| 54 | - | fake := &offlineSnapshotFake{deleteLegacyErr: injected} | |
| 55 | - | originalFactory := newOfflineSnapshotRepository | |
| 56 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |
| 57 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |
| 58 | - | ||
| 59 | - | err = runKill(context.Background(), "named", false, false) | |
| 60 | - | require.ErrorIs(t, err, injected) | |
| 61 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |
| 62 | - | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |
| 63 | - | require.NoError(t, err) | |
| 64 | - | require.Len(t, records, 1, "a failed source delete must retain retry identity") | |
| 65 | - | ||
| 66 | - | fake.deleteLegacyErr = nil | |
| 67 | - | require.NoError(t, runKill(context.Background(), "named", false, false)) | |
| 68 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) | |
| 69 | - | records, err = persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |
| 70 | - | require.NoError(t, err) | |
| 71 | - | require.Empty(t, records) | |
| 72 | - | } | |
| 73 | - | ||
| 74 | - | func TestRunKillOfflineRetriesAfterIncrementalDeleteFailure(t *testing.T) { | |
| 75 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | |
| 76 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | |
| 77 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | |
| 78 | - | ||
| 79 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | |
| 80 | - | require.NoError(t, err) | |
| 81 | - | now := time.Now().UnixNano() | |
| 82 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | |
| 83 | - | require.NoError(t, p.Close()) | |
| 84 | - | ||
| 85 | - | injected := errors.New("incremental quarantine failed") | |
| 86 | - | fake := &offlineSnapshotFake{deleteErr: injected} | |
| 87 | - | originalFactory := newOfflineSnapshotRepository | |
| 88 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |
| 89 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |
| 90 | - | ||
| 91 | - | require.ErrorIs(t, runKill(context.Background(), "named", false, false), injected) | |
| 92 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |
| 93 | - | ||
| 94 | - | fake.deleteErr = nil | |
| 95 | - | require.NoError(t, runKill(context.Background(), "named", false, false)) | |
| 96 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) | |
| 97 | - | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |
| 98 | - | require.NoError(t, err) | |
| 99 | - | require.Empty(t, records) | |
| 100 | - | } | |
| 101 | - | ||
| 102 | - | func TestRunKillOfflineReportsBothSnapshotDeletionFailures(t *testing.T) { | |
| 103 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | |
| 104 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | |
| 105 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | |
| 106 | - | ||
| 107 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | |
| 108 | - | require.NoError(t, err) | |
| 109 | - | now := time.Now().UnixNano() | |
| 110 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | |
| 111 | - | require.NoError(t, p.Close()) | |
| 112 | - | ||
| 113 | - | incrementalErr := errors.New("incremental quarantine failed") | |
| 114 | - | legacyErr := errors.New("legacy unlink failed") | |
| 115 | - | fake := &offlineSnapshotFake{deleteErr: incrementalErr, deleteLegacyErr: legacyErr} | |
| 116 | - | originalFactory := newOfflineSnapshotRepository | |
| 117 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |
| 118 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |
| 119 | - | ||
| 120 | - | err = runKill(context.Background(), "named", false, false) | |
| 121 | - | require.ErrorIs(t, err, incrementalErr) | |
| 122 | - | require.ErrorIs(t, err, legacyErr) | |
| 123 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |
| 42 | + | func TestRunKillOfflineRetriesSnapshotDeletion(t *testing.T) { | |
| 43 | + | for _, tt := range []struct { | |
| 44 | + | name string | |
| 45 | + | incrementalErr error | |
| 46 | + | legacyErr error | |
| 47 | + | }{ | |
| 48 | + | {name: "legacy failure", legacyErr: errors.New("legacy unlink failed")}, | |
| 49 | + | {name: "incremental failure", incrementalErr: errors.New("incremental quarantine failed")}, | |
| 50 | + | {name: "both failures", incrementalErr: errors.New("incremental quarantine failed"), legacyErr: errors.New("legacy unlink failed")}, | |
| 51 | + | } { | |
| 52 | + | t.Run(tt.name, func(t *testing.T) { | |
| 53 | + | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | |
| 54 | + | t.Setenv("XDG_STATE_HOME", stateRoot) | |
| 55 | + | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | |
| 56 | + | ||
| 57 | + | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | |
| 58 | + | require.NoError(t, err) | |
| 59 | + | now := time.Now().UnixNano() | |
| 60 | + | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | |
| 61 | + | require.NoError(t, p.Close()) | |
| 62 | + | ||
| 63 | + | fake := &offlineSnapshotFake{deleteErr: tt.incrementalErr, deleteLegacyErr: tt.legacyErr} | |
| 64 | + | originalFactory := newOfflineSnapshotRepository | |
| 65 | + | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |
| 66 | + | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |
| 67 | + | ||
| 68 | + | err = runKill(context.Background(), "named", false, false) | |
| 69 | + | if tt.incrementalErr != nil { | |
| 70 | + | require.ErrorIs(t, err, tt.incrementalErr) | |
| 71 | + | } | |
| 72 | + | if tt.legacyErr != nil { | |
| 73 | + | require.ErrorIs(t, err, tt.legacyErr) | |
| 74 | + | } | |
| 75 | + | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |
| 76 | + | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |
| 77 | + | require.NoError(t, err) | |
| 78 | + | require.Len(t, records, 1, "a failed source delete must retain retry identity") | |
| 79 | + | ||
| 80 | + | fake.deleteErr, fake.deleteLegacyErr = nil, nil | |
| 81 | + | require.NoError(t, runKill(context.Background(), "named", false, false)) | |
| 82 | + | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) | |
| 83 | + | records, err = persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |
| 84 | + | require.NoError(t, err) | |
| 85 | + | require.Empty(t, records) | |
| 86 | + | }) | |
| 87 | + | } | |
| 124 | 88 | } | |
| @@ -39,86 +39,50 @@ func (f *offlineSnapshotFake) DeleteTombstone(context.Context, string) error { | |||||
| 39 | return f.deleteTombstoneErr | 39 | return f.deleteTombstoneErr | ||
| 40 | } | 40 | } | ||
| 41 | 41 | ||||
| 42 | - | func TestRunKillOfflineRetainsTombstoneAndMetadataUntilBothSourcesAreDeleted(t *testing.T) { | 42 | + | func TestRunKillOfflineRetriesSnapshotDeletion(t *testing.T) { |
| 43 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | 43 | + | for _, tt := range []struct { |
| 44 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | 44 | + | name string |
| 45 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | 45 | + | incrementalErr error |
| 46 | - | 46 | + | legacyErr error | |
| 47 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | 47 | + | }{ |
| 48 | - | require.NoError(t, err) | 48 | + | {name: "legacy failure", legacyErr: errors.New("legacy unlink failed")}, |
| 49 | - | now := time.Now().UnixNano() | 49 | + | {name: "incremental failure", incrementalErr: errors.New("incremental quarantine failed")}, |
| 50 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | 50 | + | {name: "both failures", incrementalErr: errors.New("incremental quarantine failed"), legacyErr: errors.New("legacy unlink failed")}, |
| 51 | - | require.NoError(t, p.Close()) | 51 | + | } { |
| 52 | - | 52 | + | t.Run(tt.name, func(t *testing.T) { | |
| 53 | - | injected := errors.New("legacy unlink failed") | 53 | + | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() |
| 54 | - | fake := &offlineSnapshotFake{deleteLegacyErr: injected} | 54 | + | t.Setenv("XDG_STATE_HOME", stateRoot) |
| 55 | - | originalFactory := newOfflineSnapshotRepository | 55 | + | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) |
| 56 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | 56 | + | |
| 57 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | 57 | + | p, err := persist.Open(filepath.Join(stateRoot, "vev")) |
| 58 | - | 58 | + | require.NoError(t, err) | |
| 59 | - | err = runKill(context.Background(), "named", false, false) | 59 | + | now := time.Now().UnixNano() |
| 60 | - | require.ErrorIs(t, err, injected) | 60 | + | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) |
| 61 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | 61 | + | require.NoError(t, p.Close()) |
| 62 | - | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | 62 | + | |
| 63 | - | require.NoError(t, err) | 63 | + | fake := &offlineSnapshotFake{deleteErr: tt.incrementalErr, deleteLegacyErr: tt.legacyErr} |
| 64 | - | require.Len(t, records, 1, "a failed source delete must retain retry identity") | 64 | + | originalFactory := newOfflineSnapshotRepository |
| 65 | - | 65 | + | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |
| 66 | - | fake.deleteLegacyErr = nil | 66 | + | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) |
| 67 | - | require.NoError(t, runKill(context.Background(), "named", false, false)) | 67 | + | |
| 68 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) | 68 | + | err = runKill(context.Background(), "named", false, false) |
| 69 | - | records, err = persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | 69 | + | if tt.incrementalErr != nil { |
| 70 | - | require.NoError(t, err) | 70 | + | require.ErrorIs(t, err, tt.incrementalErr) |
| 71 | - | require.Empty(t, records) | 71 | + | } |
| 72 | - | } | 72 | + | if tt.legacyErr != nil { |
| 73 | - | 73 | + | require.ErrorIs(t, err, tt.legacyErr) | |
| 74 | - | func TestRunKillOfflineRetriesAfterIncrementalDeleteFailure(t *testing.T) { | 74 | + | } |
| 75 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | 75 | + | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) |
| 76 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | 76 | + | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) |
| 77 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | 77 | + | require.NoError(t, err) |
| 78 | - | 78 | + | require.Len(t, records, 1, "a failed source delete must retain retry identity") | |
| 79 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | 79 | + | |
| 80 | - | require.NoError(t, err) | 80 | + | fake.deleteErr, fake.deleteLegacyErr = nil, nil |
| 81 | - | now := time.Now().UnixNano() | 81 | + | require.NoError(t, runKill(context.Background(), "named", false, false)) |
| 82 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | 82 | + | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) |
| 83 | - | require.NoError(t, p.Close()) | 83 | + | records, err = persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) |
| 84 | - | 84 | + | require.NoError(t, err) | |
| 85 | - | injected := errors.New("incremental quarantine failed") | 85 | + | require.Empty(t, records) |
| 86 | - | fake := &offlineSnapshotFake{deleteErr: injected} | 86 | + | }) |
| 87 | - | originalFactory := newOfflineSnapshotRepository | 87 | + | } |
| 88 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |||
| 89 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |||
| 90 | - | ||||
| 91 | - | require.ErrorIs(t, runKill(context.Background(), "named", false, false), injected) | |||
| 92 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |||
| 93 | - | ||||
| 94 | - | fake.deleteErr = nil | |||
| 95 | - | require.NoError(t, runKill(context.Background(), "named", false, false)) | |||
| 96 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "delete tombstone"}, fake.calls) | |||
| 97 | - | records, err := persist.LoadReadOnly(filepath.Join(stateRoot, "vev")) | |||
| 98 | - | require.NoError(t, err) | |||
| 99 | - | require.Empty(t, records) | |||
| 100 | - | } | |||
| 101 | - | ||||
| 102 | - | func TestRunKillOfflineReportsBothSnapshotDeletionFailures(t *testing.T) { | |||
| 103 | - | stateRoot, runtimeRoot := t.TempDir(), t.TempDir() | |||
| 104 | - | t.Setenv("XDG_STATE_HOME", stateRoot) | |||
| 105 | - | t.Setenv("XDG_RUNTIME_DIR", runtimeRoot) | |||
| 106 | - | ||||
| 107 | - | p, err := persist.Open(filepath.Join(stateRoot, "vev")) | |||
| 108 | - | require.NoError(t, err) | |||
| 109 | - | now := time.Now().UnixNano() | |||
| 110 | - | require.NoError(t, p.Save(persist.Record{Name: "named", Cwd: t.TempDir(), CreatedAt: now, UpdatedAt: now})) | |||
| 111 | - | require.NoError(t, p.Close()) | |||
| 112 | - | ||||
| 113 | - | incrementalErr := errors.New("incremental quarantine failed") | |||
| 114 | - | legacyErr := errors.New("legacy unlink failed") | |||
| 115 | - | fake := &offlineSnapshotFake{deleteErr: incrementalErr, deleteLegacyErr: legacyErr} | |||
| 116 | - | originalFactory := newOfflineSnapshotRepository | |||
| 117 | - | newOfflineSnapshotRepository = func(string) offlineSnapshotSource { return fake } | |||
| 118 | - | t.Cleanup(func() { newOfflineSnapshotRepository = originalFactory }) | |||
| 119 | - | ||||
| 120 | - | err = runKill(context.Background(), "named", false, false) | |||
| 121 | - | require.ErrorIs(t, err, incrementalErr) | |||
| 122 | - | require.ErrorIs(t, err, legacyErr) | |||
| 123 | - | require.Equal(t, []string{"tombstone", "incremental", "legacy"}, fake.calls) | |||
| 124 | } | 88 | } | ||
M internal/usecase/daemon/performance_benchmark_test.go +5 -1
| @@ -76,8 +76,12 @@ func TestDaemonSnapshotDoesNotResupplyUnchangedTenThousandChunkHistory(t *testin | |||
| 76 | 76 | fixture.sess.snapshotMu.Unlock() | |
| 77 | 77 | fixture.snapshots.reset() | |
| 78 | 78 | fixture.activePane.mu.Lock() | |
| 79 | - | fixture.activePane.screen.Write([]byte("\\x1b[1;1Hchanged")) | |
| 79 | + | fixture.activePane.screen.Write([]byte("\x1b[1;1Hchanged")) | |
| 80 | + | frame := fixture.activePane.screen.PrimaryVisibleFrame() | |
| 80 | 81 | fixture.activePane.mu.Unlock() | |
| 82 | + | for i, r := range "changed" { | |
| 83 | + | require.Equalf(t, r, frame.At(i, 0).Rune, "fixture must overwrite at column %d", i) | |
| 84 | + | } | |
| 81 | 85 | markSnapshotDirty(fixture.sess) | |
| 82 | 86 | require.True(t, fixture.d.scheduleFinalSnapshot(fixture.sess)) | |
| 83 | 87 | awaitSnapshotClean(t, fixture.sess) | |
| @@ -76,8 +76,12 @@ func TestDaemonSnapshotDoesNotResupplyUnchangedTenThousandChunkHistory(t *testin | |||||
| 76 | fixture.sess.snapshotMu.Unlock() | 76 | fixture.sess.snapshotMu.Unlock() | ||
| 77 | fixture.snapshots.reset() | 77 | fixture.snapshots.reset() | ||
| 78 | fixture.activePane.mu.Lock() | 78 | fixture.activePane.mu.Lock() | ||
| 79 | - | fixture.activePane.screen.Write([]byte("\\x1b[1;1Hchanged")) | 79 | + | fixture.activePane.screen.Write([]byte("\x1b[1;1Hchanged")) |
| 80 | + | frame := fixture.activePane.screen.PrimaryVisibleFrame() | |||
| 80 | fixture.activePane.mu.Unlock() | 81 | fixture.activePane.mu.Unlock() | ||
| 82 | + | for i, r := range "changed" { | |||
| 83 | + | require.Equalf(t, r, frame.At(i, 0).Rune, "fixture must overwrite at column %d", i) | |||
| 84 | + | } | |||
| 81 | markSnapshotDirty(fixture.sess) | 85 | markSnapshotDirty(fixture.sess) | ||
| 82 | require.True(t, fixture.d.scheduleFinalSnapshot(fixture.sess)) | 86 | require.True(t, fixture.d.scheduleFinalSnapshot(fixture.sess)) | ||
| 83 | awaitSnapshotClean(t, fixture.sess) | 87 | awaitSnapshotClean(t, fixture.sess) | ||
M internal/usecase/daemon/snapshot_coordinator.go +1 -1
| @@ -208,7 +208,7 @@ func (d *Daemon) finishSnapshotCapture(capture *snapshotCapture, succeeded bool) | |||
| 208 | 208 | return | |
| 209 | 209 | } | |
| 210 | 210 | capture.finishOnce.Do(func() { | |
| 211 | - | shouldScheduleForcedSuccessor := false | |
| 211 | + | var shouldScheduleForcedSuccessor bool | |
| 212 | 212 | capture.session.snapshotMu.Lock() | |
| 213 | 213 | if capture.session.snapshotPendingCaptures > 0 { | |
| 214 | 214 | capture.session.snapshotPendingCaptures-- | |
| @@ -208,7 +208,7 @@ func (d *Daemon) finishSnapshotCapture(capture *snapshotCapture, succeeded bool) | |||||
| 208 | return | 208 | return | ||
| 209 | } | 209 | } | ||
| 210 | capture.finishOnce.Do(func() { | 210 | capture.finishOnce.Do(func() { | ||
| 211 | - | shouldScheduleForcedSuccessor := false | 211 | + | var shouldScheduleForcedSuccessor bool |
| 212 | capture.session.snapshotMu.Lock() | 212 | capture.session.snapshotMu.Lock() | ||
| 213 | if capture.session.snapshotPendingCaptures > 0 { | 213 | if capture.session.snapshotPendingCaptures > 0 { | ||
| 214 | capture.session.snapshotPendingCaptures-- | 214 | capture.session.snapshotPendingCaptures-- | ||
M internal/usecase/daemon/snapshot_import_test.go +25 -0
| @@ -76,6 +76,31 @@ func legacyAcceptanceBlob(t *testing.T, snapshot snapcodec.Session) ports.Legacy | |||
| 76 | 76 | return ports.LegacySnapshot{Name: snapshot.Name, Data: encoded} | |
| 77 | 77 | } | |
| 78 | 78 | ||
| 79 | + | func TestLegacyPublicationMarshalObjectErrorsIdentifyPaneAndStage(t *testing.T) { | |
| 80 | + | for _, tt := range []struct { | |
| 81 | + | name string | |
| 82 | + | mutate func(*snapcodec.Pane) | |
| 83 | + | stage string | |
| 84 | + | }{ | |
| 85 | + | {name: "sealed chunk", mutate: func(pane *snapcodec.Pane) { pane.SealedChunks = [][]byte{nil} }, stage: "sealed chunk 0"}, | |
| 86 | + | {name: "tail", mutate: func(pane *snapcodec.Pane) { pane.Tail = nil }, stage: "tail"}, | |
| 87 | + | {name: "visible", mutate: func(pane *snapcodec.Pane) { pane.Visible = nil }, stage: "visible"}, | |
| 88 | + | } { | |
| 89 | + | t.Run(tt.name, func(t *testing.T) { | |
| 90 | + | pane := snapcodec.Pane{ID: "pane-1", StableID: "pane-stable", Tail: []byte("tail"), Visible: []byte("visible")} | |
| 91 | + | tt.mutate(&pane) | |
| 92 | + | snapshot := snapcodec.Session{Name: "legacy", Tabs: []snapcodec.Tab{{StableID: "tab-1", Panes: []snapcodec.Pane{pane}}}} | |
| 93 | + | ||
| 94 | + | _, err := legacyPublication(snapshot) | |
| 95 | + | require.Error(t, err) | |
| 96 | + | require.ErrorContains(t, err, `tab "tab-1"`) | |
| 97 | + | require.ErrorContains(t, err, `pane "pane-1"`) | |
| 98 | + | require.ErrorContains(t, err, tt.stage) | |
| 99 | + | require.ErrorContains(t, err, "marshal object") | |
| 100 | + | }) | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 79 | 104 | func TestImportLegacyPublishesCompleteGenerationBeforeDeletingSource(t *testing.T) { | |
| 80 | 105 | snapshot := legacyAcceptanceSnapshot(t) | |
| 81 | 106 | repository := &snapshotAcceptanceRepository{generations: make(map[string]ports.SnapshotGeneration)} | |
| @@ -76,6 +76,31 @@ func legacyAcceptanceBlob(t *testing.T, snapshot snapcodec.Session) ports.Legacy | |||||
| 76 | return ports.LegacySnapshot{Name: snapshot.Name, Data: encoded} | 76 | return ports.LegacySnapshot{Name: snapshot.Name, Data: encoded} | ||
| 77 | } | 77 | } | ||
| 78 | 78 | ||||
| 79 | + | func TestLegacyPublicationMarshalObjectErrorsIdentifyPaneAndStage(t *testing.T) { | |||
| 80 | + | for _, tt := range []struct { | |||
| 81 | + | name string | |||
| 82 | + | mutate func(*snapcodec.Pane) | |||
| 83 | + | stage string | |||
| 84 | + | }{ | |||
| 85 | + | {name: "sealed chunk", mutate: func(pane *snapcodec.Pane) { pane.SealedChunks = [][]byte{nil} }, stage: "sealed chunk 0"}, | |||
| 86 | + | {name: "tail", mutate: func(pane *snapcodec.Pane) { pane.Tail = nil }, stage: "tail"}, | |||
| 87 | + | {name: "visible", mutate: func(pane *snapcodec.Pane) { pane.Visible = nil }, stage: "visible"}, | |||
| 88 | + | } { | |||
| 89 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 90 | + | pane := snapcodec.Pane{ID: "pane-1", StableID: "pane-stable", Tail: []byte("tail"), Visible: []byte("visible")} | |||
| 91 | + | tt.mutate(&pane) | |||
| 92 | + | snapshot := snapcodec.Session{Name: "legacy", Tabs: []snapcodec.Tab{{StableID: "tab-1", Panes: []snapcodec.Pane{pane}}}} | |||
| 93 | + | ||||
| 94 | + | _, err := legacyPublication(snapshot) | |||
| 95 | + | require.Error(t, err) | |||
| 96 | + | require.ErrorContains(t, err, `tab "tab-1"`) | |||
| 97 | + | require.ErrorContains(t, err, `pane "pane-1"`) | |||
| 98 | + | require.ErrorContains(t, err, tt.stage) | |||
| 99 | + | require.ErrorContains(t, err, "marshal object") | |||
| 100 | + | }) | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | ||||
| 79 | func TestImportLegacyPublishesCompleteGenerationBeforeDeletingSource(t *testing.T) { | 104 | func TestImportLegacyPublishesCompleteGenerationBeforeDeletingSource(t *testing.T) { | ||
| 80 | snapshot := legacyAcceptanceSnapshot(t) | 105 | snapshot := legacyAcceptanceSnapshot(t) | ||
| 81 | repository := &snapshotAcceptanceRepository{generations: make(map[string]ports.SnapshotGeneration)} | 106 | repository := &snapshotAcceptanceRepository{generations: make(map[string]ports.SnapshotGeneration)} | ||
M internal/usecase/daemon/snapshot_incremental.go +11 -4
| @@ -137,10 +137,7 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho | |||
| 137 | 137 | objects = append(objects, *object) | |
| 138 | 138 | } | |
| 139 | 139 | } | |
| 140 | - | tail, err := vt.MarshalHistory(pane.tail) | |
| 141 | - | if pane.tail.Len() == 0 { | |
| 142 | - | tail, err = vt.MarshalEmptyHistoryTail() | |
| 143 | - | } | |
| 140 | + | tail, err := marshalSnapshotTail(pane.tail) | |
| 144 | 141 | if err != nil { | |
| 145 | 142 | return ports.SnapshotPublication{}, err | |
| 146 | 143 | } | |
| @@ -166,6 +163,16 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho | |||
| 166 | 163 | return ports.SnapshotPublication{Name: capture.name, Generation: capture.generation, Manifest: encoded, Objects: objects}, nil | |
| 167 | 164 | } | |
| 168 | 165 | ||
| 166 | + | // marshalSnapshotTail selects the canonical empty-tail encoding before any | |
| 167 | + | // general history encoding. An empty tail is represented by its dedicated | |
| 168 | + | // canonical blob; a non-empty copied tail retains its rows. | |
| 169 | + | func marshalSnapshotTail(tail vt.HistoryView) ([]byte, error) { | |
| 170 | + | if tail.Len() == 0 { | |
| 171 | + | return vt.MarshalEmptyHistoryTail() | |
| 172 | + | } | |
| 173 | + | return vt.MarshalHistory(tail) | |
| 174 | + | } | |
| 175 | + | ||
| 169 | 176 | func prepareSnapshotChunkCache(capture *snapshotCapture) error { | |
| 170 | 177 | sess := capture.session | |
| 171 | 178 | sess.snapshotMu.Lock() | |
| @@ -137,10 +137,7 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho | |||||
| 137 | objects = append(objects, *object) | 137 | objects = append(objects, *object) | ||
| 138 | } | 138 | } | ||
| 139 | } | 139 | } | ||
| 140 | - | tail, err := vt.MarshalHistory(pane.tail) | 140 | + | tail, err := marshalSnapshotTail(pane.tail) |
| 141 | - | if pane.tail.Len() == 0 { | |||
| 142 | - | tail, err = vt.MarshalEmptyHistoryTail() | |||
| 143 | - | } | |||
| 144 | if err != nil { | 141 | if err != nil { | ||
| 145 | return ports.SnapshotPublication{}, err | 142 | return ports.SnapshotPublication{}, err | ||
| 146 | } | 143 | } | ||
| @@ -166,6 +163,16 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho | |||||
| 166 | return ports.SnapshotPublication{Name: capture.name, Generation: capture.generation, Manifest: encoded, Objects: objects}, nil | 163 | return ports.SnapshotPublication{Name: capture.name, Generation: capture.generation, Manifest: encoded, Objects: objects}, nil | ||
| 167 | } | 164 | } | ||
| 168 | 165 | ||||
| 166 | + | // marshalSnapshotTail selects the canonical empty-tail encoding before any | |||
| 167 | + | // general history encoding. An empty tail is represented by its dedicated | |||
| 168 | + | // canonical blob; a non-empty copied tail retains its rows. | |||
| 169 | + | func marshalSnapshotTail(tail vt.HistoryView) ([]byte, error) { | |||
| 170 | + | if tail.Len() == 0 { | |||
| 171 | + | return vt.MarshalEmptyHistoryTail() | |||
| 172 | + | } | |||
| 173 | + | return vt.MarshalHistory(tail) | |||
| 174 | + | } | |||
| 175 | + | ||||
| 169 | func prepareSnapshotChunkCache(capture *snapshotCapture) error { | 176 | func prepareSnapshotChunkCache(capture *snapshotCapture) error { | ||
| 170 | sess := capture.session | 177 | sess := capture.session | ||
| 171 | sess.snapshotMu.Lock() | 178 | sess.snapshotMu.Lock() | ||
M internal/usecase/daemon/snapshot_incremental_test.go +30 -0
| @@ -11,6 +11,36 @@ import ( | |||
| 11 | 11 | "github.com/bnema/vev/pkg/vt" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | + | func TestMarshalSnapshotTailSelectsCanonicalEncoder(t *testing.T) { | |
| 15 | + | for _, tt := range []struct { | |
| 16 | + | name string | |
| 17 | + | fill bool | |
| 18 | + | }{ | |
| 19 | + | {name: "empty tail"}, | |
| 20 | + | {name: "non-empty tail", fill: true}, | |
| 21 | + | } { | |
| 22 | + | t.Run(tt.name, func(t *testing.T) { | |
| 23 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: 2, ChunkRows: 2}) | |
| 24 | + | if tt.fill { | |
| 25 | + | require.NoError(t, history.Append([]renderer.Cell{{Rune: 'x'}})) | |
| 26 | + | } | |
| 27 | + | tail := history.SnapshotView().Tail() | |
| 28 | + | ||
| 29 | + | got, err := marshalSnapshotTail(tail) | |
| 30 | + | require.NoError(t, err) | |
| 31 | + | if tt.fill { | |
| 32 | + | want, err := vt.MarshalHistory(tail) | |
| 33 | + | require.NoError(t, err) | |
| 34 | + | require.Equal(t, want, got) | |
| 35 | + | return | |
| 36 | + | } | |
| 37 | + | want, err := vt.MarshalEmptyHistoryTail() | |
| 38 | + | require.NoError(t, err) | |
| 39 | + | require.Equal(t, want, got) | |
| 40 | + | }) | |
| 41 | + | } | |
| 42 | + | } | |
| 43 | + | ||
| 14 | 44 | func TestIncrementalPublicationEncodesAfterPaneUnlock(t *testing.T) { | |
| 15 | 45 | d := New(nil, nil, nil) | |
| 16 | 46 | sess := newSnapshotTestSession(t, "work", false, "/work") | |
| @@ -11,6 +11,36 @@ import ( | |||||
| 11 | "github.com/bnema/vev/pkg/vt" | 11 | "github.com/bnema/vev/pkg/vt" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | + | func TestMarshalSnapshotTailSelectsCanonicalEncoder(t *testing.T) { | |||
| 15 | + | for _, tt := range []struct { | |||
| 16 | + | name string | |||
| 17 | + | fill bool | |||
| 18 | + | }{ | |||
| 19 | + | {name: "empty tail"}, | |||
| 20 | + | {name: "non-empty tail", fill: true}, | |||
| 21 | + | } { | |||
| 22 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 23 | + | history := vt.NewHistory(vt.HistoryConfig{MaxRows: 2, ChunkRows: 2}) | |||
| 24 | + | if tt.fill { | |||
| 25 | + | require.NoError(t, history.Append([]renderer.Cell{{Rune: 'x'}})) | |||
| 26 | + | } | |||
| 27 | + | tail := history.SnapshotView().Tail() | |||
| 28 | + | ||||
| 29 | + | got, err := marshalSnapshotTail(tail) | |||
| 30 | + | require.NoError(t, err) | |||
| 31 | + | if tt.fill { | |||
| 32 | + | want, err := vt.MarshalHistory(tail) | |||
| 33 | + | require.NoError(t, err) | |||
| 34 | + | require.Equal(t, want, got) | |||
| 35 | + | return | |||
| 36 | + | } | |||
| 37 | + | want, err := vt.MarshalEmptyHistoryTail() | |||
| 38 | + | require.NoError(t, err) | |||
| 39 | + | require.Equal(t, want, got) | |||
| 40 | + | }) | |||
| 41 | + | } | |||
| 42 | + | } | |||
| 43 | + | ||||
| 14 | func TestIncrementalPublicationEncodesAfterPaneUnlock(t *testing.T) { | 44 | func TestIncrementalPublicationEncodesAfterPaneUnlock(t *testing.T) { | ||
| 15 | d := New(nil, nil, nil) | 45 | d := New(nil, nil, nil) | ||
| 16 | sess := newSnapshotTestSession(t, "work", false, "/work") | 46 | sess := newSnapshotTestSession(t, "work", false, "/work") | ||
M internal/usecase/daemon/snapshot_legacy_import.go +4 -4
| @@ -14,21 +14,21 @@ func legacyPublication(snapshot snapcodec.Session) (ports.SnapshotPublication, e | |||
| 14 | 14 | outTab := snapcodec.ManifestTab{StableID: tab.StableID, Cols: tab.Cols, Rows: tab.Rows, NextPaneID: tab.NextPaneID, Focus: tab.Focus, Tree: tab.Tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.Panes))} | |
| 15 | 15 | for _, pane := range tab.Panes { | |
| 16 | 16 | outPane := snapcodec.ManifestPane{ID: pane.ID, StableID: pane.StableID, Cwd: pane.Cwd, Process: pane.Process} | |
| 17 | - | for _, payload := range pane.SealedChunks { | |
| 17 | + | for i, payload := range pane.SealedChunks { | |
| 18 | 18 | object, err := snapcodec.MarshalObject(snapcodec.HistoryChunk, payload) | |
| 19 | 19 | if err != nil { | |
| 20 | - | return ports.SnapshotPublication{}, err | |
| 20 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q sealed chunk %d: marshal object: %w", tab.StableID, pane.ID, i, err) | |
| 21 | 21 | } | |
| 22 | 22 | outPane.Sealed = append(outPane.Sealed, objectRef(snapcodec.HistoryChunk, object)) | |
| 23 | 23 | objects = append(objects, object) | |
| 24 | 24 | } | |
| 25 | 25 | tail, err := snapcodec.MarshalObject(snapcodec.HistoryTail, pane.Tail) | |
| 26 | 26 | if err != nil { | |
| 27 | - | return ports.SnapshotPublication{}, err | |
| 27 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q tail: marshal object: %w", tab.StableID, pane.ID, err) | |
| 28 | 28 | } | |
| 29 | 29 | visible, err := snapcodec.MarshalObject(snapcodec.Visible, pane.Visible) | |
| 30 | 30 | if err != nil { | |
| 31 | - | return ports.SnapshotPublication{}, err | |
| 31 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q visible: marshal object: %w", tab.StableID, pane.ID, err) | |
| 32 | 32 | } | |
| 33 | 33 | outPane.Tail, outPane.Visible = objectRef(snapcodec.HistoryTail, tail), objectRef(snapcodec.Visible, visible) | |
| 34 | 34 | objects = append(objects, tail, visible) | |
| @@ -14,21 +14,21 @@ func legacyPublication(snapshot snapcodec.Session) (ports.SnapshotPublication, e | |||||
| 14 | outTab := snapcodec.ManifestTab{StableID: tab.StableID, Cols: tab.Cols, Rows: tab.Rows, NextPaneID: tab.NextPaneID, Focus: tab.Focus, Tree: tab.Tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.Panes))} | 14 | outTab := snapcodec.ManifestTab{StableID: tab.StableID, Cols: tab.Cols, Rows: tab.Rows, NextPaneID: tab.NextPaneID, Focus: tab.Focus, Tree: tab.Tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.Panes))} | ||
| 15 | for _, pane := range tab.Panes { | 15 | for _, pane := range tab.Panes { | ||
| 16 | outPane := snapcodec.ManifestPane{ID: pane.ID, StableID: pane.StableID, Cwd: pane.Cwd, Process: pane.Process} | 16 | outPane := snapcodec.ManifestPane{ID: pane.ID, StableID: pane.StableID, Cwd: pane.Cwd, Process: pane.Process} | ||
| 17 | - | for _, payload := range pane.SealedChunks { | 17 | + | for i, payload := range pane.SealedChunks { |
| 18 | object, err := snapcodec.MarshalObject(snapcodec.HistoryChunk, payload) | 18 | object, err := snapcodec.MarshalObject(snapcodec.HistoryChunk, payload) | ||
| 19 | if err != nil { | 19 | if err != nil { | ||
| 20 | - | return ports.SnapshotPublication{}, err | 20 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q sealed chunk %d: marshal object: %w", tab.StableID, pane.ID, i, err) |
| 21 | } | 21 | } | ||
| 22 | outPane.Sealed = append(outPane.Sealed, objectRef(snapcodec.HistoryChunk, object)) | 22 | outPane.Sealed = append(outPane.Sealed, objectRef(snapcodec.HistoryChunk, object)) | ||
| 23 | objects = append(objects, object) | 23 | objects = append(objects, object) | ||
| 24 | } | 24 | } | ||
| 25 | tail, err := snapcodec.MarshalObject(snapcodec.HistoryTail, pane.Tail) | 25 | tail, err := snapcodec.MarshalObject(snapcodec.HistoryTail, pane.Tail) | ||
| 26 | if err != nil { | 26 | if err != nil { | ||
| 27 | - | return ports.SnapshotPublication{}, err | 27 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q tail: marshal object: %w", tab.StableID, pane.ID, err) |
| 28 | } | 28 | } | ||
| 29 | visible, err := snapcodec.MarshalObject(snapcodec.Visible, pane.Visible) | 29 | visible, err := snapcodec.MarshalObject(snapcodec.Visible, pane.Visible) | ||
| 30 | if err != nil { | 30 | if err != nil { | ||
| 31 | - | return ports.SnapshotPublication{}, err | 31 | + | return ports.SnapshotPublication{}, fmt.Errorf("snapshot legacy import: tab %q pane %q visible: marshal object: %w", tab.StableID, pane.ID, err) |
| 32 | } | 32 | } | ||
| 33 | outPane.Tail, outPane.Visible = objectRef(snapcodec.HistoryTail, tail), objectRef(snapcodec.Visible, visible) | 33 | outPane.Tail, outPane.Visible = objectRef(snapcodec.HistoryTail, tail), objectRef(snapcodec.Visible, visible) | ||
| 34 | objects = append(objects, tail, visible) | 34 | objects = append(objects, tail, visible) | ||