diff --git a/SPEC.md b/SPEC.md index 3c2582f..09fbb07 100644 --- a/SPEC.md +++ b/SPEC.md @@ -815,6 +815,58 @@ Four decisions, each taken over a plausible alternative: the preset menus rather than growing its own copy of the ownership handling those exist for. +### Phase 4s.1 - Config shared between instances ✅ done +Two qtxterm windows are two processes over one `cron.json` and one +`presets.json`. Each loaded its file once at launch, so a job added in one +was invisible to the other until restart - and the next save from the stale +window wrote its old list over the new one, losing the job silently. + +- [x] **Polled once a minute, not watched.** The scheduler's tick already + runs every second, so checking on the minute costs a `stat()` and *no + extra wakeup*. A `QFileSystemWatcher` costs a worker thread, wakes on + every change in the config directory, and stops watching a file that + is *replaced* rather than modified - which is exactly what the atomic + save below does. A minute is also cron's own resolution: there is + nothing to do with a job noticed sooner than it could first fire. +- [x] **Stamped, so an unchanged file is never re-parsed.** `(mtime_ns, + size)`, compared before reading. Reloading unconditionally would emit + `changed` every minute and rebuild every menu, including one the user + has open. +- [x] **Presets are polled too, not just jobs.** A job added elsewhere + normally arrives with the Macro it names, and a job whose Macro has + not been re-read fails every single firing with "no Macro named ... + any more". +- [x] **Atomic save**, via a sibling temp file and `os.replace`. The reader + is another process polling this exact path; a plain write is visible in + its truncated middle. +- [x] **Reloads are suspended while an editor is open.** Both editors address + entries by index, and an `exec()` dialog still runs the event loop - a + poll landing mid-edit would shift the list under the open form and save + the user's changes onto whichever entry inherited that index. Counted, + not a flag, since two editors can be open over one store. +- [x] **A bad read changes nothing and is retried.** Invalid JSON - a + hand-edit in progress, a half-synced cloud drive, an older qtxterm + without the atomic save - leaves the in-memory list alone and does not + advance the stamp. Raising out of a timer would take down the window + over a file that is very likely fine a second later. A *deleted* file + is treated the same way: the presets do not evaporate mid-session. + +Two things this deliberately does not fix: + +- **The clobber window is narrowed, not closed.** Saves still write the whole + file, so an edit made in two windows inside the same minute still resolves + last-writer-wins. Closing it properly means merging per entry, which needs + stable ids the files do not have. +- **Both instances still fire the same job**, each into its own tab, since + each runs its own scheduler. Cron is per-instance by design. If that ever + needs to change, the fix is a single-instance lock at launch, not a + fourth thing for the poll to do. + +Implementation note: `config_store.py` holds this once and both stores +inherit it - `PresetStore` and `CronStore` were already the same problem +twice (a list of dataclasses, one JSON file, editors addressing entries by +index), and the file handling is now the part they literally share. + Implementation notes: - `cron.py` is schedules and storage and knows nothing about terminals; @@ -822,7 +874,8 @@ Implementation notes: what let the whole expression layer be tested without a Qt widget. - The scheduler ticks every second and acts only when the *minute* changes. A 60s timer drifts against the wall clock and skips a minute whenever the - machine sleeps. + machine sleeps. On each minute change it re-reads the config first, then + fires - see Phase 4s.1. - `next_run()` walks minute by minute but skips whole days that cannot match, and gives up after four years - "0 0 31 2 *" parses fine and can never happen, and the alternative to a bound is a UI that hangs. diff --git a/src/qtxterm/assets/USAGE.md b/src/qtxterm/assets/USAGE.md index 20d0952..b145cd3 100644 --- a/src/qtxterm/assets/USAGE.md +++ b/src/qtxterm/assets/USAGE.md @@ -364,7 +364,7 @@ that menu, the same way Macros do - useful once you have a job per feed per session. Ungrouped jobs stay at the top level. **Run Now** in the editor runs a job immediately, which beats waiting until 2am to find out whether it works. -Two things to know: +Three things to know: - **Jobs only run while qtxterm is open**, and nothing missed while it was closed is caught up on. Launching after a weekend does not fire a burst of @@ -372,6 +372,11 @@ Two things to know: machine, use the system's own scheduler. - **A job names its Macro.** Rename or delete that Macro and the job says so in the status bar rather than running something else. +- **Two qtxterm windows share one set of jobs.** Add or edit a job in one and + the other picks it up within a minute, no restart needed. Note that each + window runs its own schedule, so a job fires once per open window, each + into its own tab. If that isn't what you want, keep one window open, or + turn the job off in the others. ## Ordering diff --git a/src/qtxterm/config_store.py b/src/qtxterm/config_store.py new file mode 100644 index 0000000..20a06d8 --- /dev/null +++ b/src/qtxterm/config_store.py @@ -0,0 +1,150 @@ +"""Shared file handling for the JSON config stores. + +`PresetStore` and `CronStore` are the same problem twice: a list of +dataclasses in one JSON file under the user's config dir, edited by dialogs +that address entries by index. This holds the parts that are identical - the +atomic write, and picking up a file a *second qtxterm instance* has written. + +On the second instance: each process loads its config once at launch and +keeps it in memory, so without this a job added in one window is invisible to +the other, and the next save from the stale window writes its old list over +the new one. Polling closes most of that gap - see `reload_if_changed`. +""" + +from __future__ import annotations + +import json +import os +from contextlib import contextmanager +from pathlib import Path + +from PySide6.QtCore import QObject, Signal + +# What we compare to decide the file changed under us. mtime alone is too +# coarse on filesystems with a 1-2s timestamp granularity (FAT, some network +# shares), where two writes in the same second look identical; the size +# catches the common case of those two writes differing in length. +_Stamp = tuple[int, int] | None + + +class ConfigStore(QObject): + """A JSON list on disk, reloaded when another process rewrites it. + + Subclasses own the shape of the payload (`_apply_payload` / + `_build_payload`) and what an absent file means (`_apply_missing`); this + class owns the bytes. + """ + + changed = Signal() + + def __init__(self, path: Path) -> None: + super().__init__() + self.path = path + self._stamp: _Stamp = None + self._suspended = 0 + + # -- subclass hooks ---------------------------------------------------- + + def _apply_payload(self, raw: list) -> None: + raise NotImplementedError + + def _build_payload(self) -> list: + raise NotImplementedError + + def _apply_missing(self) -> None: + """Called by load() when the file does not exist yet.""" + raise NotImplementedError + + # -- file handling ----------------------------------------------------- + + def load(self) -> None: + if not self.path.exists(): + self._stamp = None + self._apply_missing() + return + self._apply_payload(json.loads(self.path.read_text(encoding="utf-8"))) + self._stamp = self._current_stamp() + + def save(self) -> None: + """Write the whole file, atomically, and announce it. + + Atomically because another instance may be polling this exact file: + a plain write is visible in its truncated middle, and the reader gets + a JSONDecodeError instead of a config. `os.replace` means a reader + sees either the old file or the new one. + """ + self.path.parent.mkdir(parents=True, exist_ok=True) + text = json.dumps(self._build_payload(), indent=2) + temp = self.path.with_name(f"{self.path.name}.{os.getpid()}.tmp") + try: + temp.write_text(text, encoding="utf-8") + os.replace(temp, self.path) + except OSError: + temp.unlink(missing_ok=True) + raise + self._stamp = self._current_stamp() + self.changed.emit() + + def reload_if_changed(self) -> bool: + """Re-read if another process has rewritten the file. Did we reload? + + Cheap enough to call on a timer: a stat, and a parse only when the + stamp moved. Callers get `changed` exactly as if the edit had been + made here, so every menu and sidebar already knows what to do with it. + + Never raises. A file that is unreadable *now* - mid-write by an older + qtxterm without the atomic save, half-synced by a cloud drive, or + hand-edited into invalid JSON - leaves the in-memory list alone and + is retried on the next poll. Throwing from a timer would take out the + window over a file that is very likely fine a second later. + """ + if self._suspended: + return False + stamp = self._current_stamp() + if stamp == self._stamp: + return False + if stamp is None: + # Deleted out from under us. Keeping what we have is the kinder + # reading: the file coming back is a reload, and until then the + # user's presets do not evaporate mid-session. + self._stamp = None + return False + try: + raw = json.loads(self.path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return False + # Stamped only on success, so a failed read is retried rather than + # remembered as done. + self._apply_payload(raw) + self._stamp = stamp + self.changed.emit() + return True + + def suspend_reload(self) -> None: + """Hold off reloads while an editor is open over this store. + + The editors address entries by index (`update(index, ...)`), and a + modal dialog still runs the event loop, so a poll landing mid-edit + would shift the list under the open form and save the user's changes + onto whichever entry inherited that index. Counted rather than a + flag, because more than one editor can be open over one store. + """ + self._suspended += 1 + + def resume_reload(self) -> None: + self._suspended = max(0, self._suspended - 1) + + @contextmanager + def reload_suspended(self): + self.suspend_reload() + try: + yield + finally: + self.resume_reload() + + def _current_stamp(self) -> _Stamp: + try: + info = self.path.stat() + except OSError: + return None + return (info.st_mtime_ns, info.st_size) diff --git a/src/qtxterm/cron.py b/src/qtxterm/cron.py index 413880a..bb492c0 100644 --- a/src/qtxterm/cron.py +++ b/src/qtxterm/cron.py @@ -16,12 +16,12 @@ from __future__ import annotations import dataclasses -import json from datetime import datetime, timedelta from pathlib import Path import platformdirs -from PySide6.QtCore import QObject, Signal + +from qtxterm.config_store import ConfigStore # (name, low, high) per field, in the order they are written. _FIELDS = [ @@ -222,22 +222,18 @@ def default_cron_path() -> Path: return Path(platformdirs.user_config_dir("qtxterm", appauthor=False)) / "cron.json" -class CronStore(QObject): +class CronStore(ConfigStore): """Loads/saves cron jobs as JSON, the same shape PresetStore uses.""" - changed = Signal() - def __init__(self, path: Path | None = None) -> None: - super().__init__() - self.path = path or default_cron_path() + super().__init__(path or default_cron_path()) self.jobs: list[CronJob] = [] self.load() - def load(self) -> None: - if not self.path.exists(): - self.jobs = [] - return - raw = json.loads(self.path.read_text(encoding="utf-8")) + def _apply_missing(self) -> None: + self.jobs = [] + + def _apply_payload(self, raw: list) -> None: # Unknown keys are dropped rather than raising: a file written by a # later version should cost you a field, not the whole app. fields = {f.name for f in dataclasses.fields(CronJob)} @@ -245,11 +241,8 @@ def load(self) -> None: CronJob(**{k: v for k, v in item.items() if k in fields}) for item in raw ] - def save(self) -> None: - self.path.parent.mkdir(parents=True, exist_ok=True) - payload = [dataclasses.asdict(job) for job in self.jobs] - self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8") - self.changed.emit() + def _build_payload(self) -> list: + return [dataclasses.asdict(job) for job in self.jobs] def add(self, job: CronJob) -> None: self.jobs.append(job) diff --git a/src/qtxterm/cron_editor.py b/src/qtxterm/cron_editor.py index 4b17604..a246822 100644 --- a/src/qtxterm/cron_editor.py +++ b/src/qtxterm/cron_editor.py @@ -64,6 +64,11 @@ def __init__( self._preset_store = preset_store self._scheduler = scheduler self._current_index: int | None = None + # This form addresses jobs by index, so the lists behind it must not + # be swapped out by a poll while it is open - see ConfigStore. + self._store.suspend_reload() + self._preset_store.suspend_reload() + self.finished.connect(self._resume_reloads) layout = QHBoxLayout(self) @@ -129,6 +134,10 @@ def __init__( if create_new: self._new_job() + def _resume_reloads(self) -> None: + self._store.resume_reload() + self._preset_store.resume_reload() + def _runnable_presets(self): """Macros only. diff --git a/src/qtxterm/cron_scheduler.py b/src/qtxterm/cron_scheduler.py index 5c21103..4405725 100644 --- a/src/qtxterm/cron_scheduler.py +++ b/src/qtxterm/cron_scheduler.py @@ -27,6 +27,9 @@ class CronScheduler(QObject): the app starts never fires, and nothing missed while it was closed is replayed. Otherwise launching after a weekend would open a burst of terminals before you had touched anything. + + Config is re-read on the minute, so a job added in another instance is + picked up here without a restart - see `refresh_config`. """ job_fired = Signal(str) @@ -73,8 +76,27 @@ def _on_tick(self) -> None: if minute == self._last_minute: return self._last_minute = minute + self.refresh_config() self.run_due_jobs(minute) + def refresh_config(self) -> None: + """Pick up jobs and Macros written by another qtxterm instance. + + Polled here rather than watched, for two reasons. This tick already + happens every second, so checking on the minute costs a stat and no + extra wakeup at all - where a QFileSystemWatcher costs a worker + thread, and silently stops watching when a file is *replaced*, which + is exactly what the atomic save in `ConfigStore` does. And a minute + is the resolution cron works at anyway; there is nothing to be done + with a job noticed sooner than the minute it could first fire. + + Presets as well as jobs, because a job added elsewhere usually + arrives with the Macro it runs, and a job whose Macro we have not + re-read fails every firing with "no Macro named ... any more". + """ + self._preset_store.reload_if_changed() + self._cron_store.reload_if_changed() + def run_due_jobs(self, minute: datetime) -> list[str]: """Fire every enabled job whose schedule matches `minute`.""" fired: list[str] = [] diff --git a/src/qtxterm/preset_editor.py b/src/qtxterm/preset_editor.py index 2d90939..0786966 100644 --- a/src/qtxterm/preset_editor.py +++ b/src/qtxterm/preset_editor.py @@ -121,6 +121,10 @@ def __init__( self.resize(620, 420) self._store = store self._current_index: int | None = None + # Addresses presets by index, so a poll must not reorder the list + # under the open form - see ConfigStore. + self._store.suspend_reload() + self.finished.connect(lambda _: self._store.resume_reload()) self._is_command = category == CATEGORY_COMMANDS self._is_macro = category == CATEGORY_MACROS self._is_selection = category == CATEGORY_SELECTION diff --git a/src/qtxterm/presets.py b/src/qtxterm/presets.py index 8b490a9..124e0a6 100644 --- a/src/qtxterm/presets.py +++ b/src/qtxterm/presets.py @@ -1,11 +1,11 @@ from __future__ import annotations import dataclasses -import json from pathlib import Path import platformdirs -from PySide6.QtCore import QObject, Signal + +from qtxterm.config_store import ConfigStore INPUT_NONE = "none" INPUT_SELECTION = "selection" @@ -173,35 +173,29 @@ def default_presets_path() -> Path: ) -class PresetStore(QObject): +class PresetStore(ConfigStore): """Loads/saves the preset list as JSON, seeding defaults on first run. Emits `changed` after every save() so any number of UI surfaces (sidebar, Macros menu) can stay in sync regardless of which one - triggered the edit, without reaching into each other. + triggered the edit, without reaching into each other - and, since + `ConfigStore` polls, regardless of which *instance* made the edit. """ - changed = Signal() - def __init__(self, path: Path | None = None) -> None: - super().__init__() - self.path = path or default_presets_path() + super().__init__(path or default_presets_path()) self.presets: list[Preset] = [] self.load() - def load(self) -> None: - if self.path.exists(): - raw = json.loads(self.path.read_text(encoding="utf-8")) - self.presets = [Preset(**item) for item in raw] - else: - self.presets = default_presets() - self.save() - - def save(self) -> None: - self.path.parent.mkdir(parents=True, exist_ok=True) - payload = [dataclasses.asdict(p) for p in self.presets] - self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8") - self.changed.emit() + def _apply_missing(self) -> None: + self.presets = default_presets() + self.save() + + def _apply_payload(self, raw: list) -> None: + self.presets = [Preset(**item) for item in raw] + + def _build_payload(self) -> list: + return [dataclasses.asdict(p) for p in self.presets] def sidebar_presets(self) -> list[Preset]: """Command-category presets opted into the sidebar. diff --git a/tests/test_config_store.py b/tests/test_config_store.py new file mode 100644 index 0000000..dcf8c3e --- /dev/null +++ b/tests/test_config_store.py @@ -0,0 +1,170 @@ +"""ConfigStore: picking up a file a second qtxterm instance has written. + +The "other instance" in these tests is a second store object over the same +path, which is exactly what two running windows are. +""" + +from __future__ import annotations + +import json + +from qtxterm.cron import CronJob, CronStore +from qtxterm.presets import Preset, PresetStore + + +def job(name: str, expression: str = "* * * * *") -> CronJob: + return CronJob(name=name, expression=expression, preset_name="Backup") + + +def test_a_job_added_by_another_instance_is_picked_up(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + + elsewhere.add(job("Nightly")) + + assert here.reload_if_changed() is True + assert [j.name for j in here.jobs] == ["Nightly"] + + +def test_reload_announces_the_change_like_a_local_edit(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + seen = [] + here.changed.connect(lambda: seen.append(len(here.jobs))) + + elsewhere.add(job("Nightly")) + here.reload_if_changed() + + assert seen == [1] + + +def test_an_unchanged_file_is_not_reparsed(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + seen = [] + store.changed.connect(seen.append) + + # The point of the stamp: polling every minute forever must not rebuild + # every menu every minute. + assert store.reload_if_changed() is False + assert store.reload_if_changed() is False + assert seen == [] + + +def test_our_own_save_does_not_look_like_someone_elses(tmp_path) -> None: + store = CronStore(path=tmp_path / "cron.json") + store.add(job("Nightly")) + + assert store.reload_if_changed() is False + + +def test_reload_is_held_off_while_an_editor_is_open(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + elsewhere.add(job("Nightly")) + + with here.reload_suspended(): + assert here.reload_if_changed() is False + assert here.jobs == [] + + assert here.reload_if_changed() is True + assert [j.name for j in here.jobs] == ["Nightly"] + + +def test_suspension_nests(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + elsewhere.add(job("Nightly")) + + with here.reload_suspended(): + with here.reload_suspended(): + pass + # Still held by the outer editor. + assert here.reload_if_changed() is False + + assert here.reload_if_changed() is True + + +def test_half_written_json_leaves_the_jobs_alone_and_is_retried(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + + path.write_text('[{"name": "Half', encoding="utf-8") + assert store.reload_if_changed() is False + assert [j.name for j in store.jobs] == ["Nightly"] + + # The stamp was not advanced, so the next poll still sees a change. + path.write_text( + json.dumps([{"name": "Whole", "expression": "* * * * *", "preset_name": "B"}]), + encoding="utf-8", + ) + assert store.reload_if_changed() is True + assert [j.name for j in store.jobs] == ["Whole"] + + +def test_a_deleted_file_does_not_wipe_the_jobs(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + + path.unlink() + + assert store.reload_if_changed() is False + assert [j.name for j in store.jobs] == ["Nightly"] + + +def test_unknown_fields_still_survive_a_reload(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + path.write_text( + json.dumps( + [ + { + "name": "Newer", + "expression": "* * * * *", + "preset_name": "B", + "from_a_later_version": True, + } + ] + ), + encoding="utf-8", + ) + + assert store.reload_if_changed() is True + assert [j.name for j in store.jobs] == ["Newer"] + + +def test_save_leaves_no_temporary_file_behind(tmp_path) -> None: + store = CronStore(path=tmp_path / "cron.json") + store.add(job("Nightly")) + + assert [p.name for p in tmp_path.iterdir()] == ["cron.json"] + + +def test_save_never_leaves_the_file_unreadable(tmp_path) -> None: + """The reason for the atomic write: another instance is polling this. + + Rewriting a long file with a short one is where a plain write is briefly + truncated on disk; os.replace has no such window. + """ + path = tmp_path / "cron.json" + store = CronStore(path=path) + for index in range(20): + store.jobs.append(job(f"Job {index}")) + store.save() + + store.jobs = [job("Only one")] + store.save() + + assert len(json.loads(path.read_text(encoding="utf-8"))) == 1 + + +def test_presets_reload_the_same_way(tmp_path) -> None: + path = tmp_path / "presets.json" + here, elsewhere = PresetStore(path=path), PresetStore(path=path) + elsewhere.add(Preset(name="Deploy", lines=["make deploy"], target="new_tab")) + + assert here.reload_if_changed() is True + assert here.presets[-1].name == "Deploy" diff --git a/tests/test_cron_editor.py b/tests/test_cron_editor.py index fbd3873..9dedad6 100644 --- a/tests/test_cron_editor.py +++ b/tests/test_cron_editor.py @@ -240,3 +240,45 @@ def test_clearing_the_group_makes_it_ungrouped(qtbot, tmp_path: Path) -> None: dialog._save_current() assert cron_store.jobs[0].group is None + + +def test_the_open_editor_holds_off_a_reload(qtbot, tmp_path: Path) -> None: + """The form addresses jobs by index, and a modal dialog still runs the + event loop - so a poll landing mid-edit would save onto the wrong job.""" + cron_store, preset_store = make_stores( + tmp_path, [CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")] + ) + cron_store.save() + dialog = CronEditorDialog(cron_store, preset_store) + qtbot.addWidget(dialog) + + elsewhere = CronStore(path=tmp_path / "cron.json") + elsewhere.jobs.insert( + 0, CronJob(name="Sneaked In", expression="* * * * *", preset_name="Backup") + ) + elsewhere.save() + + assert cron_store.reload_if_changed() is False + assert [job.name for job in cron_store.jobs] == ["Nightly"] + + dialog.reject() + + assert cron_store.reload_if_changed() is True + assert [job.name for job in cron_store.jobs] == ["Sneaked In", "Nightly"] + + +def test_the_open_editor_holds_off_a_preset_reload_too(qtbot, tmp_path: Path) -> None: + """Its Macro combo is built once, so the preset list must hold still too.""" + cron_store, preset_store = make_stores(tmp_path) + preset_store.save() + dialog = CronEditorDialog(cron_store, preset_store) + qtbot.addWidget(dialog) + + elsewhere = PresetStore(path=tmp_path / "presets.json") + elsewhere.add(Preset(name="Deploy", lines=["make deploy"], target="new_tab")) + + assert preset_store.reload_if_changed() is False + + dialog.reject() + + assert preset_store.reload_if_changed() is True diff --git a/tests/test_cron_scheduler.py b/tests/test_cron_scheduler.py index 73dbd5b..1b2736f 100644 --- a/tests/test_cron_scheduler.py +++ b/tests/test_cron_scheduler.py @@ -194,3 +194,85 @@ def test_run_now_ignores_the_schedule(tmp_path) -> None: assert scheduler.run_now(job) is True assert tabs.fed == [(tabs.open_tabs[0], ["echo hi"])] + + +def test_a_job_added_by_another_instance_fires_without_a_restart(tmp_path) -> None: + """The whole point of polling: two windows open, job added in the other.""" + cron_path, preset_path = tmp_path / "cron.json", tmp_path / "presets.json" + preset_store = PresetStore(path=preset_path) + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + preset_store.save() + scheduler = CronScheduler(CronStore(path=cron_path), preset_store, FakeTabs()) + + elsewhere = CronStore(path=cron_path) + elsewhere.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == ["Nightly"] + + +def test_a_macro_added_by_another_instance_arrives_with_its_job(tmp_path) -> None: + """A job is useless here without the Macro it names, so both are polled.""" + cron_path, preset_path = tmp_path / "cron.json", tmp_path / "presets.json" + preset_store = PresetStore(path=preset_path) + preset_store.presets = [] + preset_store.save() + cron_store = CronStore(path=cron_path) + scheduler = CronScheduler(cron_store, preset_store, FakeTabs()) + failures = [] + scheduler.job_failed.connect(lambda name, why: failures.append((name, why))) + + other_presets = PresetStore(path=preset_path) + other_presets.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + other_presets.save() + other_cron = CronStore(path=cron_path) + other_cron.add( + CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup") + ) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == ["Nightly"] + assert failures == [] + + +def test_a_job_deleted_elsewhere_stops_firing(tmp_path) -> None: + cron_path = tmp_path / "cron.json" + preset_store = PresetStore(path=tmp_path / "presets.json") + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + cron_store = CronStore(path=cron_path) + cron_store.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + scheduler = CronScheduler(cron_store, preset_store, FakeTabs()) + + elsewhere = CronStore(path=cron_path) + elsewhere.delete(0) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == [] + + +def test_a_reload_keeps_a_running_job_pointed_at_its_own_tab(tmp_path) -> None: + """Tabs are keyed by job name, so re-reading the file must not orphan one.""" + cron_path = tmp_path / "cron.json" + preset_store = PresetStore(path=tmp_path / "presets.json") + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + cron_store = CronStore(path=cron_path) + cron_store.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + tabs = FakeTabs() + scheduler = CronScheduler(cron_store, preset_store, tabs) + + scheduler.run_due_jobs(at("2026-08-17 02:00")) + first_tab = tabs.open_tabs[0] + + # Another instance re-saves the same job with a changed schedule. + elsewhere = CronStore(path=cron_path) + elsewhere.update( + 0, CronJob(name="Nightly", expression="0 3 * * *", preset_name="Backup") + ) + scheduler.refresh_config() + scheduler.run_due_jobs(at("2026-08-18 03:00")) + + assert tabs.open_tabs == [first_tab] + assert [terminal for terminal, _ in tabs.fed] == [first_tab, first_tab] diff --git a/tests/test_preset_editor.py b/tests/test_preset_editor.py index 9f9e017..254bc1b 100644 --- a/tests/test_preset_editor.py +++ b/tests/test_preset_editor.py @@ -450,3 +450,27 @@ def test_the_new_order_is_saved_and_shown_in_the_menu(qtbot, tmp_path: Path) -> menu = MacrosMenu(store, tabs) listed = [a.text() for a in menu.actions()][:2] assert listed == ["Second", "First"] + + +def test_the_open_editor_holds_off_a_reload(qtbot, tmp_path) -> None: + """Presets are addressed by index here, so the list must not be swapped + out by a poll while the form is open.""" + store = PresetStore(path=tmp_path / "presets.json") + store.presets = [Preset(name="Build", lines=["make"], target="active")] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_COMMANDS) + qtbot.addWidget(dialog) + + elsewhere = PresetStore(path=tmp_path / "presets.json") + elsewhere.presets.insert( + 0, Preset(name="Sneaked In", lines=["x"], target="active") + ) + elsewhere.save() + + assert store.reload_if_changed() is False + assert [p.name for p in store.presets] == ["Build"] + + dialog.reject() + + assert store.reload_if_changed() is True + assert [p.name for p in store.presets] == ["Sneaked In", "Build"]