Fix #13: Expose Pre-edit Backup restoration
Preserve full diff opcodes before grouping, route modal navigation to a two-axis viewport-sized scroller, and calculate side-by-side layout using terminal cell widths so tabbed Hosts File lines keep a stable divider.
This commit is contained in:
parent
6a3e1e0d5a
commit
e268794564
13 changed files with 1103 additions and 19 deletions
|
|
@ -1,19 +1,22 @@
|
|||
"""Tests for application-level authorization flow."""
|
||||
|
||||
from contextlib import contextmanager
|
||||
import asyncio
|
||||
from io import StringIO
|
||||
import os
|
||||
import signal
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
from rich.cells import cell_len
|
||||
from textual.app import SuspendNotSupported
|
||||
from textual.widgets import Input, RadioButton, Static
|
||||
from textual.widgets import Button, Input, RadioButton, Static
|
||||
|
||||
from src.hosts.core.filters import FilterOptions
|
||||
from src.hosts.core.models import HostEntry, HostsFile
|
||||
from src.hosts.tui.app import HostsManagerApp
|
||||
from src.hosts.tui.custom_footer import CustomFooter
|
||||
from src.hosts.tui.backup_restore_modal import BackupRestoreModal
|
||||
from src.hosts.tui.filter_modal import FilterModal
|
||||
from src.hosts.tui.help_modal import HelpModal
|
||||
from src.hosts.tui.privilege_prompt import (
|
||||
|
|
@ -246,6 +249,147 @@ async def test_small_viewport_blocks_hidden_mutation_shortcuts():
|
|||
app.manager.enter_edit_mode.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_opens_a_side_by_side_preview_with_exact_paths(tmp_path):
|
||||
"""The recovery workflow makes its before-and-after files reviewable."""
|
||||
app = app_with_filterable_entries()
|
||||
backup_path = tmp_path / "hosts.backup"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
backup_path.write_text("127.0.0.1 restored.test\n")
|
||||
hosts_file_path.write_text("127.0.0.1 current.test\n")
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
app.action_restore_backup()
|
||||
await pilot.pause()
|
||||
await pilot.pause()
|
||||
|
||||
assert isinstance(app.screen, BackupRestoreModal)
|
||||
assert str(backup_path) in str(
|
||||
app.screen.query_one("#backup-restored-path", Static).render()
|
||||
)
|
||||
assert str(hosts_file_path) in str(
|
||||
app.screen.query_one("#backup-current-path", Static).render()
|
||||
)
|
||||
diff = str(app.screen.query_one("#backup-diff", Static).render())
|
||||
assert "current.test" in diff
|
||||
assert "restored.test" in diff
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_side_by_side_divider_uses_terminal_cell_widths(tmp_path):
|
||||
"""Tabs in Hosts File lines cannot move the right-hand file between rows."""
|
||||
app = app_with_filterable_entries()
|
||||
backup_path = tmp_path / "hosts.backup"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
hosts_file_path.write_text(
|
||||
"127.0.0.1\tlocalhost\n"
|
||||
"255.255.255.255\tbroadcasthost\n"
|
||||
"::1\tlocalhost\n"
|
||||
"192.0.2.10\tcurrent.test\n"
|
||||
)
|
||||
backup_path.write_text(
|
||||
"127.0.0.1\tlocalhost\n"
|
||||
"255.255.255.255\tbroadcasthost\n"
|
||||
"::1\tlocalhost\n"
|
||||
"192.0.2.10\trestored.test\n"
|
||||
)
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
|
||||
async with app.run_test(size=(160, 40)) as pilot:
|
||||
app.action_restore_backup()
|
||||
await pilot.pause()
|
||||
await pilot.pause()
|
||||
|
||||
modal = app.screen
|
||||
assert isinstance(modal, BackupRestoreModal)
|
||||
lines = [
|
||||
line for line in modal._render_diff().plain.splitlines() if " │ " in line
|
||||
]
|
||||
divider_columns = {
|
||||
cell_len(line[: line.index("│")].expandtabs(8)) for line in lines
|
||||
}
|
||||
|
||||
assert "\t" not in "\n".join(lines)
|
||||
assert len(divider_columns) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_preview_disables_a_no_change_operation(tmp_path):
|
||||
"""A no-op restore cannot perform an unnecessary privileged write."""
|
||||
app = app_with_filterable_entries()
|
||||
backup_path = tmp_path / "hosts.backup"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
contents = "127.0.0.1 localhost\n"
|
||||
backup_path.write_text(contents)
|
||||
hosts_file_path.write_text(contents)
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
app.action_restore_backup()
|
||||
await pilot.pause()
|
||||
await pilot.pause()
|
||||
|
||||
modal = app.screen
|
||||
assert isinstance(modal, BackupRestoreModal)
|
||||
assert modal.query_one("#restore-button", Button).disabled
|
||||
assert "would make no changes" in str(
|
||||
modal.query_one("#backup-restore-status", Static).render()
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_preview_fills_width_and_routes_navigation_from_cancel(tmp_path):
|
||||
"""The safe initial focus does not prevent full mode or two-axis scrolling."""
|
||||
app = app_with_filterable_entries()
|
||||
backup_path = tmp_path / "hosts.backup"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
current_lines = [
|
||||
f"192.0.2.{index} host-{index}.test " + "x" * 120 for index in range(1, 81)
|
||||
]
|
||||
restored_lines = current_lines.copy()
|
||||
restored_lines[39] = "192.0.2.40 restored.test " + "y" * 120
|
||||
hosts_file_path.write_text("\n".join(current_lines) + "\n")
|
||||
backup_path.write_text("\n".join(restored_lines) + "\n")
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
|
||||
async with app.run_test(size=(160, 40)) as pilot:
|
||||
app.action_restore_backup()
|
||||
await pilot.pause()
|
||||
await pilot.pause()
|
||||
|
||||
modal = app.screen
|
||||
assert isinstance(modal, BackupRestoreModal)
|
||||
container = modal.query_one("#backup-restore-container")
|
||||
assert container.outer_size.width == 156
|
||||
|
||||
changed_diff = str(modal.query_one("#backup-diff", Static).render())
|
||||
await pilot.press("f")
|
||||
await pilot.pause()
|
||||
full_diff = str(modal.query_one("#backup-diff", Static).render())
|
||||
assert full_diff != changed_diff
|
||||
assert "host-1.test" in full_diff
|
||||
|
||||
scroller = modal.query_one("#backup-diff-scroll")
|
||||
assert app.focused is modal.query_one("#cancel-button")
|
||||
await pilot.press("down", "right")
|
||||
await pilot.pause()
|
||||
assert scroller.scroll_y > 0
|
||||
assert scroller.scroll_x > 0
|
||||
|
||||
|
||||
class TestPrivilegedModeAuthorization:
|
||||
"""Test the user-visible privileged-mode authorization flow."""
|
||||
|
||||
|
|
@ -474,3 +618,80 @@ class TestPrivilegedModeAuthorization:
|
|||
assert not app.manager.permission_manager._sudo_validated
|
||||
mock_run.assert_not_called()
|
||||
app.update_status.assert_called_once_with("Edit mode disabled")
|
||||
|
||||
|
||||
class TestPreEditBackupRestoration:
|
||||
"""Test the user-reachable Pre-edit Backup recovery flow."""
|
||||
|
||||
def test_restore_action_confirms_paths_then_reloads_after_success(self, tmp_path):
|
||||
app = HostsManagerApp()
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
backup_path = tmp_path / "hosts.backup.123"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
backup_path.write_text("127.0.0.1 restored.test\n")
|
||||
hosts_file_path.write_text("127.0.0.1 current.test\n")
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
app.manager.restore_backup.return_value = (True, "Backup restored successfully")
|
||||
app.push_screen = Mock()
|
||||
app.load_hosts_file = Mock()
|
||||
app.update_status = Mock()
|
||||
|
||||
app.run_worker = Mock(side_effect=lambda coroutine, **_: asyncio.run(coroutine))
|
||||
app.action_restore_backup()
|
||||
|
||||
modal, callback = app.push_screen.call_args.args
|
||||
assert modal.preview.backup_path == backup_path
|
||||
assert modal.preview.hosts_file_path == hosts_file_path
|
||||
|
||||
callback(True)
|
||||
|
||||
app.manager.restore_backup.assert_called_once_with(
|
||||
modal.preview.backup_digest, modal.preview.hosts_file_digest
|
||||
)
|
||||
app.load_hosts_file.assert_called_once_with()
|
||||
assert app.update_status.call_args.args == (
|
||||
f"Pre-edit Backup restored from {backup_path} to {hosts_file_path}",
|
||||
)
|
||||
|
||||
def test_failed_restore_keeps_interface_state_and_reports_paths(self, tmp_path):
|
||||
app = HostsManagerApp()
|
||||
app.edit_mode = True
|
||||
app.manager = Mock()
|
||||
backup_path = tmp_path / "hosts.backup.123"
|
||||
hosts_file_path = tmp_path / "hosts"
|
||||
backup_path.write_text("127.0.0.1 restored.test\n")
|
||||
hosts_file_path.write_text("127.0.0.1 current.test\n")
|
||||
app.manager.backup_path = backup_path
|
||||
app.manager.parser.file_path = hosts_file_path
|
||||
app.manager.restore_backup.return_value = (False, "Permission denied")
|
||||
app.push_screen = Mock()
|
||||
app.load_hosts_file = Mock()
|
||||
app.update_status = Mock()
|
||||
|
||||
app.run_worker = Mock(side_effect=lambda coroutine, **_: asyncio.run(coroutine))
|
||||
app.action_restore_backup()
|
||||
_, callback = app.push_screen.call_args.args
|
||||
callback(True)
|
||||
|
||||
app.load_hosts_file.assert_not_called()
|
||||
assert app.update_status.call_args.args == (
|
||||
f"Failed to restore Pre-edit Backup from {backup_path} to {hosts_file_path}: Permission denied. "
|
||||
"The Hosts File and interface are unchanged. Check write access and try again.",
|
||||
)
|
||||
|
||||
def test_restore_is_unavailable_while_the_entry_editor_has_unsaved_state(self):
|
||||
app = HostsManagerApp()
|
||||
app.entry_edit_mode = True
|
||||
app.manager = Mock()
|
||||
app.push_screen = Mock()
|
||||
app.update_status = Mock()
|
||||
|
||||
app.action_restore_backup()
|
||||
|
||||
app.push_screen.assert_not_called()
|
||||
app.manager.restore_backup.assert_not_called()
|
||||
app.update_status.assert_called_once_with(
|
||||
"Finish or cancel the Entry Editor before restoring the Pre-edit Backup."
|
||||
)
|
||||
|
|
|
|||
70
tests/test_backup_restore_modal.py
Normal file
70
tests/test_backup_restore_modal.py
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
"""Tests for the Pre-edit Backup restoration confirmation."""
|
||||
|
||||
import asyncio
|
||||
from unittest.mock import Mock
|
||||
|
||||
from textual.widgets import Button
|
||||
|
||||
from hosts.tui.backup_restore_modal import BackupRestoreModal
|
||||
from hosts.core.restore_preview import create_restore_preview
|
||||
|
||||
|
||||
async def unchanged_preview(preview):
|
||||
"""Return the already reviewed files for modal unit tests."""
|
||||
return preview
|
||||
|
||||
|
||||
class TestBackupRestoreModal:
|
||||
"""The modal identifies both paths and requires an explicit choice."""
|
||||
|
||||
def test_modal_exposes_backup_and_hosts_file_paths(self, tmp_path):
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_text("127.0.0.1 localhost\n")
|
||||
hosts_file.write_text("192.0.2.1 current.test\n")
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview))
|
||||
|
||||
assert modal.preview.backup_path == backup
|
||||
assert modal.preview.hosts_file_path == hosts_file
|
||||
|
||||
def test_confirm_dismisses_true_after_freshness_check(self, tmp_path):
|
||||
backup = tmp_path / "backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_text("127.0.0.1 restored.test\n")
|
||||
hosts_file.write_text("127.0.0.1 current.test\n")
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview))
|
||||
modal.dismiss = Mock()
|
||||
modal._set_status = Mock()
|
||||
|
||||
asyncio.run(modal._confirm())
|
||||
|
||||
modal.dismiss.assert_called_once_with(True)
|
||||
|
||||
def test_cancel_is_focused_and_dismisses_false(self):
|
||||
preview = Mock()
|
||||
modal = BackupRestoreModal(preview, Mock())
|
||||
modal.dismiss = Mock()
|
||||
cancel = Mock()
|
||||
modal.query_one = Mock(return_value=cancel)
|
||||
modal._render_preview = Mock()
|
||||
|
||||
modal.on_mount()
|
||||
modal.action_cancel()
|
||||
|
||||
modal.query_one.assert_called_once_with("#cancel-button", Button)
|
||||
cancel.focus.assert_called_once_with()
|
||||
modal.dismiss.assert_called_once_with(False)
|
||||
|
||||
def test_final_newline_difference_is_visible_without_changed_lines(self, tmp_path):
|
||||
backup = tmp_path / "backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_bytes(b"127.0.0.1 localhost\n")
|
||||
hosts_file.write_bytes(b"127.0.0.1 localhost")
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
modal = BackupRestoreModal(preview, lambda: unchanged_preview(preview))
|
||||
|
||||
rendered = modal._render_diff()
|
||||
|
||||
assert "No newline at end of current Hosts File" in str(rendered)
|
||||
|
|
@ -13,6 +13,7 @@ from unittest.mock import Mock, patch
|
|||
from src.hosts.core.manager import PermissionManager, HostsManager
|
||||
from src.hosts.core.models import HostEntry, HostsFile
|
||||
from src.hosts.core.commands import ToggleEntryCommand
|
||||
from src.hosts.core.restore_preview import fingerprint_file
|
||||
|
||||
|
||||
class TestPermissionManager:
|
||||
|
|
@ -592,15 +593,69 @@ class TestHostsManager:
|
|||
manager._backup_path = Path(backup_file.name)
|
||||
|
||||
try:
|
||||
manager.undo_redo_history.clear_history = Mock()
|
||||
success, message = manager.restore_backup()
|
||||
|
||||
assert success
|
||||
assert "restored successfully" in message
|
||||
mock_run.assert_called_once()
|
||||
assert mock_run.call_count == 2
|
||||
assert mock_run.call_args_list[0].args[0][:2] == ["sudo", "cp"]
|
||||
assert mock_run.call_args_list[0].args[0][-1] != temp_file.name
|
||||
assert mock_run.call_args_list[1].args[0][:2] == ["sudo", "mv"]
|
||||
assert mock_run.call_args_list[1].args[0][-1] == temp_file.name
|
||||
manager.undo_redo_history.clear_history.assert_called_once_with()
|
||||
finally:
|
||||
# Clean up
|
||||
manager._backup_path.unlink()
|
||||
|
||||
@patch("subprocess.run")
|
||||
def test_restore_backup_copy_failure_leaves_hosts_file_target_untouched(
|
||||
self, mock_run
|
||||
):
|
||||
"""A failed staged copy never opens the Hosts File target for writing."""
|
||||
mock_run.return_value = Mock(returncode=1, stderr="Disk full")
|
||||
|
||||
with tempfile.NamedTemporaryFile() as temp_file:
|
||||
manager = HostsManager(temp_file.name)
|
||||
manager.edit_mode = True
|
||||
with tempfile.NamedTemporaryFile(delete=False) as backup_file:
|
||||
manager._backup_path = Path(backup_file.name)
|
||||
|
||||
try:
|
||||
success, message = manager.restore_backup()
|
||||
|
||||
assert not success
|
||||
assert "Disk full" in message
|
||||
mock_run.assert_called_once()
|
||||
assert mock_run.call_args.args[0][-1] != temp_file.name
|
||||
finally:
|
||||
manager._backup_path.unlink()
|
||||
|
||||
@patch("subprocess.run")
|
||||
def test_restore_backup_refuses_files_that_changed_since_preview(self, mock_run):
|
||||
"""The privileged write uses the exact source and target the user reviewed."""
|
||||
with tempfile.NamedTemporaryFile() as temp_file:
|
||||
manager = HostsManager(temp_file.name)
|
||||
manager.edit_mode = True
|
||||
with tempfile.NamedTemporaryFile(delete=False) as backup_file:
|
||||
backup_file.write(b"127.0.0.1 restored.test\n")
|
||||
manager._backup_path = Path(backup_file.name)
|
||||
|
||||
try:
|
||||
backup_digest = fingerprint_file(
|
||||
manager._backup_path, "Pre-edit Backup"
|
||||
)
|
||||
hosts_digest = fingerprint_file(manager.parser.file_path, "Hosts File")
|
||||
Path(temp_file.name).write_text("192.0.2.2 changed.test\n")
|
||||
|
||||
success, message = manager.restore_backup(backup_digest, hosts_digest)
|
||||
|
||||
assert not success
|
||||
assert "changed since review" in message
|
||||
mock_run.assert_not_called()
|
||||
finally:
|
||||
manager._backup_path.unlink()
|
||||
|
||||
def test_restore_backup_not_in_edit_mode(self):
|
||||
"""Test restoring backup when not in edit mode."""
|
||||
with tempfile.NamedTemporaryFile() as temp_file:
|
||||
|
|
|
|||
73
tests/test_restore_preview.py
Normal file
73
tests/test_restore_preview.py
Normal file
|
|
@ -0,0 +1,73 @@
|
|||
"""Tests for the exact-file preview shown before Pre-edit Backup restoration."""
|
||||
|
||||
import pytest
|
||||
|
||||
from hosts.core.restore_preview import RestorePreviewError, create_restore_preview
|
||||
|
||||
|
||||
def test_preview_pairs_current_and_restored_lines_in_changed_hunks(tmp_path):
|
||||
"""The current Hosts File is the left/before side of the preview."""
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_text("127.0.0.1 localhost\n192.0.2.1 restored.test\n")
|
||||
hosts_file.write_text("127.0.0.1 localhost\n192.0.2.2 current.test\n")
|
||||
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
|
||||
assert preview.has_changes
|
||||
row = preview.changed_hunks[0].rows[1]
|
||||
assert row.current_line_number == 2
|
||||
assert row.restored_line_number == 2
|
||||
assert row.current_text == "192.0.2.2 current.test"
|
||||
assert row.restored_text == "192.0.2.1 restored.test"
|
||||
|
||||
|
||||
def test_preview_distinguishes_missing_final_newlines(tmp_path):
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_bytes(b"127.0.0.1 localhost\n")
|
||||
hosts_file.write_bytes(b"127.0.0.1 localhost")
|
||||
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
|
||||
assert preview.has_changes
|
||||
assert preview.restored_has_final_newline
|
||||
assert not preview.current_has_final_newline
|
||||
|
||||
|
||||
def test_preview_rejects_files_that_cannot_be_decoded_as_hosts_text(tmp_path):
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_bytes(b"\xff")
|
||||
hosts_file.write_text("127.0.0.1 localhost\n")
|
||||
|
||||
with pytest.raises(RestorePreviewError, match="Pre-edit Backup"):
|
||||
create_restore_preview(backup, hosts_file)
|
||||
|
||||
|
||||
def test_preview_detects_when_either_reviewed_file_changes(tmp_path):
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
backup.write_text("127.0.0.1 localhost\n")
|
||||
hosts_file.write_text("192.0.2.2 current.test\n")
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
|
||||
hosts_file.write_text("192.0.2.3 changed.test\n")
|
||||
|
||||
assert not preview.matches_files()
|
||||
|
||||
|
||||
def test_full_preview_keeps_lines_outside_changed_hunk_context(tmp_path):
|
||||
"""Full mode retains the complete files after changed hunks are calculated."""
|
||||
backup = tmp_path / "hosts.backup"
|
||||
hosts_file = tmp_path / "hosts"
|
||||
current_lines = [f"192.0.2.{index} host-{index}.test" for index in range(1, 41)]
|
||||
restored_lines = current_lines.copy()
|
||||
restored_lines[19] = "192.0.2.20 restored.test"
|
||||
hosts_file.write_text("\n".join(current_lines) + "\n")
|
||||
backup.write_text("\n".join(restored_lines) + "\n")
|
||||
|
||||
preview = create_restore_preview(backup, hosts_file)
|
||||
|
||||
assert len(preview.changed_hunks[0].rows) == 7
|
||||
assert len(preview.full_hunks[0].rows) == 40
|
||||
Loading…
Add table
Add a link
Reference in a new issue