From 14513cc898a84e797d9d562b8f6ce6602fba2210 Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:19:24 +0200 Subject: [PATCH 1/5] Speed up OCI Modify entry by deferring backup probes and reusing verified config --- .../test_oci_update_preservation_wording.py | 1 + .github/workflows/test-oci-registries.yml | 4 + oci/src/proxmenux_oci/management.py | 29 ++++- oci/tests/test_modify_latency.py | 100 ++++++++++++++++++ 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 oci/tests/test_modify_latency.py diff --git a/.github/scripts/tests/test_oci_update_preservation_wording.py b/.github/scripts/tests/test_oci_update_preservation_wording.py index 44146970..477196e9 100644 --- a/.github/scripts/tests/test_oci_update_preservation_wording.py +++ b/.github/scripts/tests/test_oci_update_preservation_wording.py @@ -130,6 +130,7 @@ class UpdateWording(unittest.TestCase): reconcile.propose = lambda *args: None scope = {'sys': SimpleNamespace(path=[], executable='python3'), 'translate': lambda text: text, 'check_selected': lambda project, row: row, + '_selected_configuration': lambda vmid: instances.command('pct', 'config', str(vmid)), 'images': SimpleNamespace(offer_removal=lambda *args: None), 'carry_records': lambda project, **options: {}, '_work_backup_ready': lambda project, ui, vmid: True, diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index 9612c0d3..7625fe1c 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.github/workflows/test-oci-registries.yml @@ -32,3 +32,7 @@ jobs: env: PYTHONDONTWRITEBYTECODE: '1' run: python3 -m unittest discover -s oci/tests -p 'test_registry_*.py' -v + - name: Run OCI Modify latency regressions + env: + PYTHONDONTWRITEBYTECODE: '1' + run: python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v diff --git a/oci/src/proxmenux_oci/management.py b/oci/src/proxmenux_oci/management.py index cbf8e041..9b71d1b1 100644 --- a/oci/src/proxmenux_oci/management.py +++ b/oci/src/proxmenux_oci/management.py @@ -106,13 +106,32 @@ def _config_hostname(vmid): return '' +def _selected_configuration(vmid): + """Reuse pct's output only while its complete native source is unchanged. + + The forced carried-record verification has just fetched a fresh config. + Older installations without that source-keyed cache still query pct. + The lifecycle backend independently validates again before any mutation. + """ + import oci_instances as instances + try: + import oci_carried_record + reader = getattr(oci_carried_record, 'configuration', None) + config = reader(instances.ROOT, vmid) if reader else None + if isinstance(config, str) and config: + return config.encode() + except (ImportError, OSError, ValueError, RuntimeError, subprocess.SubprocessError): + pass + return instances.command('pct', 'config', str(vmid)) + + def check_selected(project, row): """Validate only the chosen container; lifecycle backends validate its stack.""" sys.path.insert(0, str(project / 'remote')) import oci_instances as instances with instances.locked(instances.ROOT): record = instances.read(instances.ROOT, row['vmid']) - config = instances.command('pct', 'config', str(row['vmid'])) + config = _selected_configuration(row['vmid']) marker = instances.identity(config) reason = 'matched' if marker == record['installation_id'] else 'identity-unconfirmed' return public_row(record, {'reason': reason}) @@ -397,12 +416,12 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()): return _toggle_watchdog(project, ui, row['vmid'], watched) if action == 'remove': return _remove(project, ui, row['vmid']) - if not _work_backup_ready(project, ui, row['vmid']): + if action != 'modify' and not _work_backup_ready(project, ui, row['vmid']): return False record = instances.read(instances.ROOT, row['vmid']) import oci_instance_reconcile as reconcile try: - current_config = instances.command('pct', 'config', str(row['vmid'])) + current_config = _selected_configuration(row['vmid']) adoption = reconcile.propose(record, current_config) except (OSError, ValueError, RuntimeError) as error: ui.message(str(error), translate('Review external OCI changes')) @@ -443,6 +462,10 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()): wizard.close() if not approved: return False + # Opening or cancelling the editor needs no disk/storage probes. + # Keep the existing backup checks after the user confirms edits. + if not _work_backup_ready(project, ui, row['vmid']): + return False elif not ui.review(translate('If the image channel has a new version, the CT is stopped, backed up and verified, and replaced with the new image to update the container. If the update fails after the container was changed, the backup is restored. An update that stops halfway is recovered from the OCI management menu.'), translate('Update OCI'), question=translate('Update now?'), default=True): return False diff --git a/oci/tests/test_modify_latency.py b/oci/tests/test_modify_latency.py new file mode 100644 index 00000000..788215ad --- /dev/null +++ b/oci/tests/test_modify_latency.py @@ -0,0 +1,100 @@ +"""Opening Modify avoids backup probes while preserving operation safeguards.""" +from pathlib import Path +import sys +import unittest +from unittest.mock import Mock,patch +ROOT=Path(__file__).resolve().parents[1] +sys.path[:0]=[str(ROOT/'src'),str(ROOT/'remote')] +from proxmenux_oci import management as menu,recreation +import oci_carried_record as carried +import oci_instances as instances +import oci_instance_reconcile as reconcile +from proxmenux_oci.ui import UserCancelled + + +def record(): + return {'vmid': 115, 'installation_id': 'test-installation', 'status': 'installed', + 'observed': {'config_sha256': 'native-config-fingerprint'}, + 'template': {'id': 'example-app', 'catalog_ui': {'title': 'Example'}, + 'container_contract': {'image': {'reference': 'example/app:latest'}}, + 'proxmox': {'installer_profile': {}}}, + 'deployment': {'vmid': 115, 'hostname': 'example', 'resources': {'cores': 2, 'memory_mb': 512}, + 'network': {'bridge': 'vmbr0', 'ipv4': 'dhcp', 'gateway': None}, + 'rootfs': {'storage': 'local-zfs', 'size_gb': 8}, 'mounts': [ + {'container_path': '/data', 'source': 'local-zfs', + 'type': 'managed-volume', 'size_gb': 8, 'backup': True}], + 'devices': [], 'security': {'unprivileged': True}, + 'environment': [{'name': 'PASSWORD', 'value': 'private-test-secret', + 'sensitive': True}]}} + + +class SelectedConfigTests(unittest.TestCase): + def test_valid_source_keyed_cache_avoids_another_pct_process(self): + with patch.object(carried,'configuration',create=True,return_value='description: current\n') as reader, \ + patch.object(instances,'command') as command: + self.assertEqual(menu._selected_configuration(115),b'description: current\n') + reader.assert_called_once_with(instances.ROOT,115) + command.assert_not_called() + + def test_old_or_unavailable_cache_falls_back_to_pct(self): + for reader in (None,Mock(return_value=None),Mock(return_value={}),Mock(side_effect=OSError('cache unavailable'))): + with self.subTest(reader=reader),patch.object(carried,'configuration',reader,create=True), \ + patch.object(instances,'command',return_value=b'live config') as command: + self.assertEqual(menu._selected_configuration(115),b'live config') + command.assert_called_once_with('pct','config','115') + + def test_identity_check_is_still_performed_on_the_selected_config(self): + r=record();r['installation_id']='11111111-1111-4111-8111-111111111111' + import contextlib + with patch.object(instances,'locked',return_value=contextlib.nullcontext()), \ + patch.object(instances,'read',return_value=r), \ + patch.object(menu,'_selected_configuration',return_value=b'description: another container\n'): + self.assertEqual(menu.check_selected(ROOT,{'vmid':115})['reason'],'identity-unconfirmed') + + +class EditorLatencyTests(unittest.TestCase): + def run_editor(self,proposal=None,backup_ready=True,stale=False,adoption_error=False,approved=True): + events=[];saved=record() + row={'vmid':115,'reason':'matched','stack':False,'pending':False,'status':'installed'} + def edit(*a,**kw): + events.append('editor') + if proposal is None:raise UserCancelled() + return proposal + def backup(*a,**kw):events.append('backup');return backup_ready + ui=Mock();ui.unattended=True;ui.review.return_value=approved + with patch.object(menu,'carry_records',return_value={115:'stale'} if stale else {}) as verify, \ + patch.object(menu,'check_selected',return_value=row), \ + patch.object(menu,'_selected_configuration',return_value=b'current config'), \ + patch.object(instances,'read',return_value=saved), \ + patch.object(reconcile,'propose',side_effect=ValueError('external changes') if adoption_error else None,return_value=None), \ + patch.object(recreation,'edit_recreation',side_effect=edit), \ + patch.object(menu,'_work_backup_ready',side_effect=backup), \ + patch.object(menu,'_run_lifecycle',side_effect=lambda *a:events.append('lifecycle') or True): + try:result=menu.manage_instance(ROOT,ui,row,action='modify') + except UserCancelled:result=False + verify.assert_called_once_with(ROOT,vmids=[115],verify=True) + return result,events + + def test_open_and_cancel_has_no_backup_or_storage_probes(self): + result,events=self.run_editor() + self.assertFalse(result);self.assertEqual(events,['editor']) + + def test_declining_the_review_has_no_backup_or_storage_probes(self): + result,events=self.run_editor({'operation':'recreate','candidate':record()},approved=False) + self.assertFalse(result);self.assertEqual(events,['editor']) + + def test_confirmed_edit_keeps_backup_check_before_the_lifecycle(self): + result,events=self.run_editor({'operation':'recreate','candidate':record()}) + self.assertTrue(result);self.assertEqual(events,['editor','backup','lifecycle']) + + def test_missing_backup_space_still_prevents_the_lifecycle(self): + result,events=self.run_editor({'operation':'recreate','candidate':record()},backup_ready=False) + self.assertFalse(result);self.assertEqual(events,['editor','backup']) + + def test_restored_or_conflicting_instance_cannot_enter_editor(self): + for options in ({'stale':True},{'adoption_error':True}): + result,events=self.run_editor(**options) + self.assertFalse(result);self.assertEqual(events,[]) + + +if __name__=='__main__':unittest.main() From 1e97f732acc807caaa372c785c3d8814babbbd3a Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:40:19 +0200 Subject: [PATCH 2/5] Avoid unnecessary probes while opening OCI menus --- .../test_oci_removal_firewall_wording.py | 6 +- .github/workflows/test-oci-registries.yml | 6 +- oci/remote/oci_work_backup.py | 16 ++- oci/src/proxmenux_oci/management.py | 17 +-- oci/tests/test_menu_performance.py | 109 ++++++++++++++++++ 5 files changed, 137 insertions(+), 17 deletions(-) create mode 100644 oci/tests/test_menu_performance.py diff --git a/.github/scripts/tests/test_oci_removal_firewall_wording.py b/.github/scripts/tests/test_oci_removal_firewall_wording.py index bde90937..8041f923 100644 --- a/.github/scripts/tests/test_oci_removal_firewall_wording.py +++ b/.github/scripts/tests/test_oci_removal_firewall_wording.py @@ -56,7 +56,8 @@ class RemovalWordings(unittest.TestCase): state = ModuleType('oci_installation_state') state.parse_config = lambda raw: {} scope = {'sys': SimpleNamespace(path=[]), 'source_text': lambda value: value or '', - 'translate': translated or (lambda value: value), 're': re} + 'translate': translated or (lambda value: value), 're': re, + '_selected_configuration': remover.guest_config, 'subprocess': subprocess} with patch.dict(sys.modules, {'oci_instances': instances, 'oci_remove': remover, 'oci_installation_state': state}): return extract(MENU, '_removal_summary', scope)(Path('/inert'), 101) @@ -87,7 +88,8 @@ class RemovalWordings(unittest.TestCase): with patch.dict(sys.modules, {'oci_instances': instances, 'oci_remove': remover, 'oci_installation_state': state}): scope = {'sys': SimpleNamespace(path=[]), 'source_text': lambda value: value or '', - 'translate': lambda value: value, 're': re} + 'translate': lambda value: value, 're': re, + '_selected_configuration': remover.guest_config, 'subprocess': subprocess} preview = extract(MENU, '_removal_summary', scope)(Path('/inert'), 102) self.assertIn(WHOLE, preview) diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index 7625fe1c..cab053e6 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.github/workflows/test-oci-registries.yml @@ -35,4 +35,8 @@ jobs: - name: Run OCI Modify latency regressions env: PYTHONDONTWRITEBYTECODE: '1' - run: python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v + run: | + python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v + python3 -m unittest discover -s oci/tests -p test_menu_performance.py -v + python3 -m unittest discover -s oci/tests -p test_work_backup.py -v + python3 -m unittest discover -s oci/tests -p test_recreation.py -v diff --git a/oci/remote/oci_work_backup.py b/oci/remote/oci_work_backup.py index b37a2c26..251ffe71 100644 --- a/oci/remote/oci_work_backup.py +++ b/oci/remote/oci_work_backup.py @@ -192,21 +192,25 @@ def application(root, vmid): return primary_id, members or [vmid] -def status(root, vmid): +def status(root, vmid, only_needed=False): """What the menu needs to know before an operation: whether the backup fits on the system disk, the storage chosen earlier and the ones that - have room.""" + have room. The menu can skip unused alternatives once a destination fits; + the default retains the complete status used by the maintenance CLI.""" primary_id, vmids = application(root, vmid) record = instances.read(root, primary_id) default = instances.location(root, primary_id).parent needed, free = required(vmids), available(default) saved = (record.get('deployment') or {}).get(KEY) - path = storage_path(saved) if saved else None + fits = free >= needed + path = storage_path(saved) if saved and not (only_needed and fits) else None saved_free = available(path) if path is not None and path.is_dir() else None - return {'needed': needed, 'free': free, 'fits': free >= needed, 'saved': saved, - 'saved_fits': saved_free is not None and saved_free >= needed, + saved_fits = saved_free is not None and saved_free >= needed + candidates = [] if only_needed and (fits or saved_fits) else file_storages() + return {'needed': needed, 'free': free, 'fits': fits, 'saved': saved, + 'saved_fits': saved_fits, 'candidates': [{'storage': item['storage'], 'free': item['free']} - for item in file_storages() if item['free'] >= needed]} + for item in candidates if item['free'] >= needed]} def choose(root, vmid, storage): diff --git a/oci/src/proxmenux_oci/management.py b/oci/src/proxmenux_oci/management.py index 9b71d1b1..db75d597 100644 --- a/oci/src/proxmenux_oci/management.py +++ b/oci/src/proxmenux_oci/management.py @@ -332,7 +332,7 @@ def _work_backup_ready(project, ui, vmid): import oci_instances as instances import oci_work_backup try: - state = oci_work_backup.status(instances.ROOT, vmid) + state = oci_work_backup.status(instances.ROOT, vmid, only_needed=True) except (OSError, ValueError, KeyError, RuntimeError, subprocess.SubprocessError): # What cannot be measured here is told by the engine when it stops. return True @@ -416,8 +416,6 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()): return _toggle_watchdog(project, ui, row['vmid'], watched) if action == 'remove': return _remove(project, ui, row['vmid']) - if action != 'modify' and not _work_backup_ready(project, ui, row['vmid']): - return False record = instances.read(instances.ROOT, row['vmid']) import oci_instance_reconcile as reconcile try: @@ -462,13 +460,13 @@ def manage_instance(project, ui, row, action=None, lifecycle_args=()): wizard.close() if not approved: return False - # Opening or cancelling the editor needs no disk/storage probes. - # Keep the existing backup checks after the user confirms edits. - if not _work_backup_ready(project, ui, row['vmid']): - return False elif not ui.review(translate('If the image channel has a new version, the CT is stopped, backed up and verified, and replaced with the new image to update the container. If the update fails after the container was changed, the backup is restored. An update that stops halfway is recovered from the OCI management menu.'), translate('Update OCI'), question=translate('Update now?'), default=True): return False + # Opening/cancelling a confirmation needs no disk or storage probes. + # Both accepted operations retain this check before the backend starts. + if not _work_backup_ready(project, ui, row['vmid']): + return False command = [sys.executable, str(project / 'remote/oci_update_current.py'), str(row['vmid']), *lifecycle_args] # Host directories are configured by the user, who knows they are @@ -518,7 +516,10 @@ def _removal_summary(project, vmid): record = instances.read(instances.ROOT, member) except (OSError, ValueError, KeyError): record = {} - config = oci_remove.guest_config(member) + try: + config = _selected_configuration(member) + except (OSError, RuntimeError, subprocess.SubprocessError): + config = None cfg = parse_config(config) if config else {} name = (cfg.get('hostname') or (record.get('stack_member') or {}).get('name') or record.get('deployment', {}).get('hostname') or '') diff --git a/oci/tests/test_menu_performance.py b/oci/tests/test_menu_performance.py new file mode 100644 index 00000000..94981ceb --- /dev/null +++ b/oci/tests/test_menu_performance.py @@ -0,0 +1,109 @@ +"""Menu speedups avoid unused probes, retaining confirmation and capacity checks.""" +from pathlib import Path +import sys +import tempfile +import unittest +from unittest.mock import Mock,patch +ROOT=Path(__file__).resolve().parents[1] +sys.path[:0]=[str(ROOT/'src'),str(ROOT/'remote')] +from proxmenux_oci import management as menu +import oci_instances as instances +import oci_instance_reconcile as reconcile +import oci_remove as remover +import oci_work_backup as backup + + +class UpdateEntryTests(unittest.TestCase): + def run_update(self,approved,ready): + events=[] + record={'status':'installed','vmid':110,'deployment':{},'template':{}} + row={'vmid':110,'reason':'matched','stack':False,'pending':False,'status':'installed'} + ui=Mock();ui.unattended=True + ui.review.side_effect=lambda *a,**kw:events.append('review') or approved + with patch.object(menu,'carry_records',return_value={}) as carry, \ + patch.object(menu,'check_selected',return_value=row), \ + patch.object(menu,'_selected_configuration',return_value=b'current config'), \ + patch.object(instances,'read',return_value=record),patch.object(reconcile,'propose',return_value=None), \ + patch.object(menu,'_work_backup_ready',side_effect=lambda *a:events.append('backup') or ready), \ + patch.object(menu,'_run_lifecycle',side_effect=lambda *a:events.append('lifecycle') or True): + result=menu.manage_instance(ROOT,ui,row,action='update') + carry.assert_called_once_with(ROOT,vmids=[110],verify=True) + return result,events + + def test_declining_update_does_not_probe_disks_or_storages(self): + result,events=self.run_update(False,False) + self.assertFalse(result);self.assertEqual(events,['review']) + + def test_accepted_update_checks_backup_space_before_starting(self): + result,events=self.run_update(True,True) + self.assertTrue(result);self.assertEqual(events,['review','backup','lifecycle']) + + def test_insufficient_space_still_prevents_the_update(self): + result,events=self.run_update(True,False) + self.assertFalse(result);self.assertEqual(events,['review','backup']) + + +class BackupDestinationTests(unittest.TestCase): + def status(self,local_free,saved_free=None,only_needed=True): + with tempfile.TemporaryDirectory() as directory: + root=Path(directory);saved= root/'nas';saved.mkdir() + record={'deployment':{backup.KEY:'nas'}} + with patch.object(backup,'application',return_value=(110,[110,111])), \ + patch.object(instances,'read',return_value=record), \ + patch.object(backup,'required',return_value=100) as required, \ + patch.object(backup,'available',side_effect=lambda p:saved_free if Path(p)==saved else local_free), \ + patch.object(backup,'storage_path',return_value=saved if saved_free is not None else None) as path, \ + patch.object(backup,'file_storages',return_value=[{'storage':'enough','free':200},{'storage':'small','free':50}]) as alternatives: + result=backup.status(root,110,only_needed=only_needed) + required.assert_called_once_with([110,111]) + return result,path.call_count,alternatives.call_count + + def test_local_room_avoids_saved_storage_and_alternative_queries(self): + result,saved,alternatives=self.status(200,saved_free=1000) + self.assertTrue(result['fits']);self.assertEqual((saved,alternatives),(0,0)) + + def test_saved_destination_that_fits_avoids_alternative_queries(self): + result,saved,alternatives=self.status(50,saved_free=200) + self.assertTrue(result['saved_fits']);self.assertEqual((saved,alternatives),(1,0)) + + def test_when_neither_fits_all_available_destinations_are_checked(self): + result,saved,alternatives=self.status(50,saved_free=40) + self.assertFalse(result['fits']);self.assertFalse(result['saved_fits']) + self.assertEqual(result['candidates'],[{'storage':'enough','free':200}]) + self.assertEqual(alternatives,1) + + def test_missing_saved_storage_still_offers_alternatives(self): + result,saved,alternatives=self.status(50) + self.assertEqual(result['candidates'],[{'storage':'enough','free':200}]) + self.assertEqual(alternatives,1) + + def test_complete_status_keeps_legacy_fields_and_all_candidates(self): + result,saved,alternatives=self.status(200,saved_free=300,only_needed=False) + self.assertTrue(result['fits']);self.assertTrue(result['saved_fits']) + self.assertEqual((saved,alternatives),(1,1)) + self.assertEqual(result['candidates'],[{'storage':'enough','free':200}]) + + +class RemovalPreviewTests(unittest.TestCase): + def preview(self,reader): + record={'vmid':110,'deployment':{},'template':{'catalog_ui':{'title':'Example'}}} + with patch.object(remover,'members_of',return_value=(110,record,[110])), \ + patch.object(instances,'read',return_value=record), \ + patch.object(menu,'_selected_configuration',side_effect=reader), \ + patch.object(remover,'guest_config',side_effect=AssertionError('Duplicate pct process')), \ + patch.object(remover,'host_directories',return_value=[]), \ + patch.object(remover,'private_bridge',return_value=None): + return menu._removal_summary(ROOT,110) + + def test_preview_reuses_verified_config_and_keeps_volume_and_host_path_details(self): + config=b'hostname: example\nrootfs: local:subvol-110-disk-0,size=8G\nmp0: /mnt/media,mp=/media,backup=0\n' + text=self.preview(lambda v:config) + self.assertIn('8G',text);self.assertIn('/mnt/media',text) + self.assertIn('not targeted for removal',text) + + def test_missing_guest_can_still_be_reviewed_without_a_second_probe(self): + text=self.preview(lambda v:(_ for _ in ()).throw(RuntimeError('guest missing'))) + self.assertIn('CT 110',text);self.assertIn('rootfs -',text) + + +if __name__=='__main__':unittest.main() From 6496318452ec950b4eaff377bef7a79b957e004d Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:47:32 +0200 Subject: [PATCH 3/5] Shorten Escape key delay in OCI dialog menus --- .github/workflows/test-oci-registries.yml | 1 + oci/src/proxmenux_oci/ui.py | 4 +++ oci/tests/test_dialog_escape.py | 34 +++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 oci/tests/test_dialog_escape.py diff --git a/.github/workflows/test-oci-registries.yml b/.github/workflows/test-oci-registries.yml index cab053e6..2d2bf69e 100644 --- a/.github/workflows/test-oci-registries.yml +++ b/.github/workflows/test-oci-registries.yml @@ -38,5 +38,6 @@ jobs: run: | python3 -m unittest discover -s oci/tests -p test_modify_latency.py -v python3 -m unittest discover -s oci/tests -p test_menu_performance.py -v + python3 -m unittest discover -s oci/tests -p test_dialog_escape.py -v python3 -m unittest discover -s oci/tests -p test_work_backup.py -v python3 -m unittest discover -s oci/tests -p test_recreation.py -v diff --git a/oci/src/proxmenux_oci/ui.py b/oci/src/proxmenux_oci/ui.py index 8238d3c2..6ff5955f 100644 --- a/oci/src/proxmenux_oci/ui.py +++ b/oci/src/proxmenux_oci/ui.py @@ -221,6 +221,10 @@ class DialogUI: def _run(self, widget: list[str], title: str | None = None) -> subprocess.CompletedProcess[str]: environment = os.environ.copy() + # ncurses otherwise waits a full second to distinguish Escape from + # the prefix of a cursor/function key. Keep a short sequence window, + # while respecting an explicit terminal-specific setting. + environment.setdefault("ESCDELAY", "150") if environment.get("TERM", "").casefold() in {"", "dumb", "unknown"}: environment["TERM"] = "xterm-256color" # dialog draws on the terminal and writes the selection to stderr. diff --git a/oci/tests/test_dialog_escape.py b/oci/tests/test_dialog_escape.py new file mode 100644 index 00000000..f5063878 --- /dev/null +++ b/oci/tests/test_dialog_escape.py @@ -0,0 +1,34 @@ +"""Escape delay is scoped to dialog and preserves terminal-specific overrides.""" +import os +from pathlib import Path +import subprocess +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'src')) +from proxmenux_oci.ui import DialogUI, UserCancelled + + +class DialogEscapeTests(unittest.TestCase): + def test_escape_uses_short_delay_without_changing_parent_environment(self): + with patch.dict(os.environ, {'TERM': 'xterm'}, clear=True), \ + patch('proxmenux_oci.ui.subprocess.run', return_value=subprocess.CompletedProcess([], 255, stderr='')) as run: + self.assertIsNone(DialogUI().choose('Menu', [('1', 'First')])) + self.assertEqual(run.call_args.kwargs['env']['ESCDELAY'], '150') + self.assertNotIn('ESCDELAY', os.environ) + + def test_explicit_delay_is_preserved(self): + with patch.dict(os.environ, {'ESCDELAY': '900'}), \ + patch('proxmenux_oci.ui.subprocess.run', return_value=subprocess.CompletedProcess([], 0, stderr='1')) as run: + self.assertEqual(DialogUI().choose('Menu', [('1', 'First')]), '1') + self.assertEqual(run.call_args.kwargs['env']['ESCDELAY'], '900') + + def test_escape_in_search_input_keeps_cancellation_behavior(self): + with patch('proxmenux_oci.ui.subprocess.run', return_value=subprocess.CompletedProcess([], 255, stderr='')): + with self.assertRaises(UserCancelled): + DialogUI().ask('Search') + + +if __name__ == '__main__': + unittest.main() From 99adc343e8805fa2d64566808a81559988cc3f9d Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:50:42 +0200 Subject: [PATCH 4/5] Adapt isolated removal preview tests to verified configuration reader --- .github/scripts/tests/test_oci_recovery_inventory_wording.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/scripts/tests/test_oci_recovery_inventory_wording.py b/.github/scripts/tests/test_oci_recovery_inventory_wording.py index 77e7afaa..338698ae 100644 --- a/.github/scripts/tests/test_oci_recovery_inventory_wording.py +++ b/.github/scripts/tests/test_oci_recovery_inventory_wording.py @@ -5,6 +5,7 @@ import json from pathlib import Path import re import runpy +import subprocess import sys import tempfile from types import ModuleType, SimpleNamespace @@ -52,7 +53,8 @@ class InventoryMessages(unittest.TestCase): state = ModuleType('oci_installation_state') state.parse_config = lambda raw: dict(line.split(': ', 1) for line in raw.decode().splitlines()) scope = {'sys': SimpleNamespace(path=[]), 'source_text': lambda text: text or '', - 'translate': translate, 're': re} + 'translate': translate, 're': re, 'subprocess': subprocess, + '_selected_configuration': remover.guest_config} summary = extracted(MENU, '_removal_summary', scope) with patch.dict(sys.modules, {'oci_instances': instances, 'oci_remove': remover, 'oci_installation_state': state}): From 32b71bdd3d236a6e2f3fce66f0b894c288772eb6 Mon Sep 17 00:00:00 2001 From: MacRimi <123239993+MacRimi@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:52:03 +0200 Subject: [PATCH 5/5] test(oci): follow the review inside the Modify menu in the latency test --- oci/tests/test_modify_latency.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/oci/tests/test_modify_latency.py b/oci/tests/test_modify_latency.py index 788215ad..f08359a5 100644 --- a/oci/tests/test_modify_latency.py +++ b/oci/tests/test_modify_latency.py @@ -53,11 +53,12 @@ class SelectedConfigTests(unittest.TestCase): class EditorLatencyTests(unittest.TestCase): - def run_editor(self,proposal=None,backup_ready=True,stale=False,adoption_error=False,approved=True): + def run_editor(self,proposal=None,backup_ready=True,stale=False,adoption_error=False,approved=True,declined=False): events=[];saved=record() row={'vmid':115,'reason':'matched','stack':False,'pending':False,'status':'installed'} def edit(*a,**kw): events.append('editor') + if declined:return None if proposal is None:raise UserCancelled() return proposal def backup(*a,**kw):events.append('backup');return backup_ready @@ -80,7 +81,9 @@ class EditorLatencyTests(unittest.TestCase): self.assertFalse(result);self.assertEqual(events,['editor']) def test_declining_the_review_has_no_backup_or_storage_probes(self): - result,events=self.run_editor({'operation':'recreate','candidate':record()},approved=False) + # The review is part of the Modify menu: declining it and leaving the + # menu returns no proposal, so no probe or operation follows. + result,events=self.run_editor(declined=True) self.assertFalse(result);self.assertEqual(events,['editor']) def test_confirmed_edit_keeps_backup_check_before_the_lifecycle(self):