From 5f175722c4671c9c6d4ffbdb5e3bf12c67dacb24 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 17:05:26 +0200 Subject: [PATCH 1/7] feat(paths): keep user files out of the install directory A package manager owns the directory it installs into. WinGet records an extracted archive's top-level directory as one entry and removes every recorded entry before installing the new ones, so a portable zip whose archive has a single top-level directory loses everything written beside the executable on each upgrade, and a portable package has no script hook a manifest could use. Chocolatey keeps files it never installed, but its package folder grants plain users read and execute only, so a non-elevated run cannot save there at all. Frozen builds now resolve the profiles, the window state, both CSV exports and the crash log against %LOCALAPPDATA%\. Running from sources is unchanged, and BEAN_DATA_DIR overrides both for a genuinely portable copy. - paths: app_dir() becomes user_data_dir(); add ensure_data_dir(), migrate_user_files() and prepare_user_data() - migration copies rather than moves, so a rollback still finds its files, and only takes a file the target lacks, so a stale copy cannot overwrite newer data. It parses nothing on the way and skips a directory carrying a user file's name - the location does not probe whether the exe directory is writable: that would make it depend on elevation, and one install would silently keep two sets of profiles - gui: prepare_user_data() runs before the first store, since both stores bind their path at import; problems are logged through _report_storage_problems - new key log.data_files_problem in both language files - the two CSV exports move to gui/csv_export.py so app.py stays under the size ratchet, which drops 1287 -> 1202 Guard: tests/test_user_data_location.py, eleven checks, with the location mutation registered in tests/test_mutation_registry.py. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 ++ README.md | 1 + README.pl.md | 1 + beantester/crashlog.py | 6 +- beantester/gui/app.py | 164 +++----------------------- beantester/gui/csv_export.py | 167 ++++++++++++++++++++++++++ beantester/paths.py | 161 ++++++++++++++++++++++--- lang/en.json | 1 + lang/pl.json | 1 + smoke_gui.py | 4 +- tests/conftest.py | 8 +- tests/test_code_shape.py | 7 +- tests/test_conns_export.py | 14 +-- tests/test_crashlog.py | 2 +- tests/test_mutation_registry.py | 10 ++ tests/test_readme_guards.py | 8 +- tests/test_release_fixes.py | 4 +- tests/test_user_data_location.py | 194 +++++++++++++++++++++++++++++++ tests/test_view_scope.py | 6 +- tests/user_files.py | 2 +- 20 files changed, 582 insertions(+), 187 deletions(-) create mode 100644 beantester/gui/csv_export.py create mode 100644 tests/test_user_data_location.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6392784..0ea0286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ignored rather than duplicated. Like every other row action they fill the form only - press "Apply changes" to put them into a running session. +### Changed +- **Your profiles, window state and CSV exports now live in your own user folder** + (`%LOCALAPPDATA%\BeanNetworkTester`) instead of the program folder. The first time you start + this version, files from an earlier one are copied over and the originals are left where they + were, so going back to an older build still finds them. The reason is updates: a package + manager owns the program folder and replaces it, which would take your saved profiles with it. + Set `BEAN_DATA_DIR` to any folder to keep everything together instead, for example on a stick. + ### Docs - **Four more guides on the website: no internet, timed scenarios, game lag and chaos testing.** How to take the internet away from one app while the local network keeps working, how a scenario diff --git a/README.md b/README.md index b6730cb..3eef99c 100644 --- a/README.md +++ b/README.md @@ -1184,6 +1184,7 @@ beantester/ the implementation package rates.py throughput averaging (a pure, testable helper) scope.py what the numbers on screen cover (one pure verdict) crash.py what the GUI tells the crash logger: report context, breadcrumb + csv_export.py the two CSV exports and the column names they write theme.py chart.py tooltip.py profiles.py icon.py labels.py lang/ translations (en, pl) tests/ pytest tests diff --git a/README.pl.md b/README.pl.md index da5eb7c..4b0b9be 100644 --- a/README.pl.md +++ b/README.pl.md @@ -1040,6 +1040,7 @@ beantester/ pakiet z implementacją rates.py uśrednianie przepustowości (czysty, testowalny helper) scope.py co obejmują liczby na ekranie (jeden czysty werdykt) crash.py co GUI mówi logowi awarii: kontekst raportu i okruszek + csv_export.py dwa eksporty CSV i nazwy kolumn, które zapisują theme.py chart.py tooltip.py profiles.py icon.py labels.py lang/ tłumaczenia (en, pl) tests/ testy pytest diff --git a/beantester/crashlog.py b/beantester/crashlog.py index 766f9b5..da628dd 100644 --- a/beantester/crashlog.py +++ b/beantester/crashlog.py @@ -65,7 +65,7 @@ from datetime import datetime, timezone from .appinfo import __version__ -from .paths import app_dir +from .paths import user_data_dir CRASH_DIR_NAME = "crashes" LOG_NAME = "crashes.ndjson" @@ -91,8 +91,8 @@ # -- where ------------------------------------------------------------------- # def crash_dir(): - """Directory the crash files live in. Next to the executable, like the profiles.""" - return os.path.join(app_dir(), CRASH_DIR_NAME) + """Directory the crash files live in - the user's data directory, by the profiles.""" + return os.path.join(user_data_dir(), CRASH_DIR_NAME) def _ensure_dir(): diff --git a/beantester/gui/app.py b/beantester/gui/app.py index e39e1d2..c30c3f5 100644 --- a/beantester/gui/app.py +++ b/beantester/gui/app.py @@ -14,7 +14,6 @@ purpose - no mixed languages), * every visible text is looked up through ``T()`` at widget-build time. """ -import csv import os import queue import sys @@ -34,9 +33,10 @@ from ..fields import FIELD_DEFS, SECTIONS, UI_ONLY_KEYS, off_value from ..filters import cli_key_for, i18n_key_for, i18n_keys, windivert_for from .. import crashlog +from . import csv_export from ..i18n import (FALLBACK_LANGUAGE, T, available_languages, current_language, field_name, set_language) -from ..paths import CONNECTIONS_CSV_FILE, CSV_FILE, scenarios_dir +from ..paths import prepare_user_data, scenarios_dir from ..presets import (PRESETS, preset_to_settings, resolve_preset, settings_to_preset) from ..processes import port_process_map @@ -47,7 +47,6 @@ settings_from_raw, warn_if_unbounded) from ..summary import settings_summary from ..utils import number_string -from ..views import avg_packet_bytes, connection_proc, filter_sort_connections from . import crash as gui_crash from . import dialogs from .icon import (apply_window_icon, make_gear_icon, show_idle_icon, @@ -105,6 +104,12 @@ def __init__(self, root): root.configure(bg=BG) apply_dark_titlebar(root) # while still hidden, so it never flashes light + # BEFORE any store reads a file: create the data directory and adopt what an + # older build left next to the executable (see paths.user_data_dir). Both + # stores bind their path at import, so this cannot wait until the log exists - + # whatever went wrong is reported by _report_storage_problems instead. + self._data_problems = prepare_user_data() + self.ui = UiStateStore() # Secondary windows live in a registry, exactly like the pages and the # settings fields do (gui/windows.py): a new window is one entry, and it @@ -1029,152 +1034,15 @@ def _persist_profiles(self): self.log(f"{T('log.profiles_not_saved')}: {err}") # -- session actions ---------------------------------------------------------- # - # Internal stat keys are engine-speak ("seen"); a CSV is read by people and - # by spreadsheets, so it gets column names that mean something. - CSV_COLUMNS = {"seen": "packets_seen", "scoped_seen": "packets_in_scope", - "drop_loss": "dropped_loss", - "drop_overflow": "dropped_overflow", "drop_syn": "dropped_syn", - "drop_mtu": "dropped_mtu", "drop_nat": "dropped_nat", - "drop_rst": "dropped_rst", "rst_reset": "connections_reset", - "drop_lan": "dropped_lan", - "drop_block": "dropped_block", - "drop_flap": "dropped_link_outage", "drop_rate": "dropped_rate_limit", - "drop_shutdown": "dropped_at_stop", - "drop_send": "dropped_send_failed", - "queue": "queue_len", - "peak_queue": "queue_peak", - # The stats CSV deliberately does NOT follow the "show only the - # targeted traffic" preference: it is an APPEND log, and a file - # whose columns mean one thing in some rows and another in the - # rest is worse than useless for the spreadsheet it exists for. - # It gains both totals instead, so the reader can do the - # narrowing themselves and see which is which. - "bytes_in_scoped": "delivered_in_scope_bytes_down", - "bytes_out_scoped": "delivered_in_scope_bytes_up"} - - # Columns that come from the SESSION rather than the counters: they say which - # world the counters were measured in. The comment above explains why this - # file must not follow the view preference - a column meaning one thing in - # some rows and another in the rest is useless to the spreadsheet it exists - # for. Capture narrowing is the STRONGER version of that problem and had no - # column at all: it does not pick between two totals, it changes `seen` - # itself, so a narrowed row and a wide row were indistinguishable while - # counting completely different traffic. The CLI has carried the same fact in - # its JSON summary (`capture_narrowed`) since narrowing shipped, and the repro - # report carries the whole `session_info` - only this file could not say. - # Keyed by session_info() key, like CSV_COLUMNS is keyed by counter key. - CSV_SESSION_COLUMNS = {"narrowed": "capture_narrowed"} - - @staticmethod - def _csv_session_value(key, info): - """One session cell. Booleans as yes/no in English, like `impaired` in the - connections export - a CSV is read by scripts and spreadsheets, so it does - not follow the interface language.""" - value = info.get(key) - return ("yes" if value else "no") if isinstance(value, bool) else value - + # The two exports live in gui/csv_export.py - they are file work, not window + # work, and this file is the one at the size ceiling (convention 32b). The + # methods stay because every caller uses them: two pages, the smoke script, + # the tests and the `_export_csv` alias at the bottom of this file. def export_csv(self): - snap = self.engine.stats_snapshot() - info = self.engine.session_info() - session_cells = [self._csv_session_value(k, info) - for k in self.CSV_SESSION_COLUMNS] - header = ["time", *self.CSV_SESSION_COLUMNS.values(), - *(self.CSV_COLUMNS.get(k, k) for k in snap)] - try: - write_header = not os.path.exists(CSV_FILE) - if not write_header: - with open(CSV_FILE, newline="", encoding="utf-8") as f: - existing = next(csv.reader(f), []) - if existing != header: - # the stat columns changed between versions: appending would - # silently misalign rows against the old header - backup = CSV_FILE[:-4] + time.strftime(".%Y%m%d-%H%M%S.csv") - os.replace(CSV_FILE, backup) - self.log(f"{T('log.csv_rotated')} {os.path.basename(backup)}") - write_header = True - with open(CSV_FILE, "a", newline="", encoding="utf-8") as f: - writer = csv.writer(f) - if write_header: - writer.writerow(header) - writer.writerow([time.strftime("%Y-%m-%d %H:%M:%S"), - *session_cells, *snap.values()]) - self.log(f"{T('log.stats_saved_to')} {os.path.basename(CSV_FILE)}") - except Exception as e: - self.log(f"{T('log.csv_error')}: {e}") - - # Mirrors the table's columns so the export is the table, on disk. Raw bytes, - # not KB: a CSV is read by a spreadsheet or a script, where exact, summable - # integers beat the one-decimal KB the table shows for people. `impaired` is - # "yes"/"no" in English, like the headers - the CSV is language-independent. - # delivered_* is what reached the application; captured_* is what the tool - # saw offered. The old download_bytes/upload_bytes/total_bytes held CAPTURED - # under names every other surface uses for delivered, so they are renamed - # rather than reused - a column that quietly changes meaning is worse than one - # that disappears. - CONN_CSV_HEADER = ["process", "pid", "proto", "remote_ip", "remote_port", - "local_port", "packets", "impaired", "dropped", - "delivered_down_bytes", "delivered_up_bytes", - "delivered_total_bytes", - "captured_down_bytes", "captured_up_bytes", - "captured_total_bytes", - "avg_bytes", "duration_s", "idle_s"] + csv_export.export_stats(self) def export_connections_csv(self): - """Write the CURRENT connection view (search + sort) to a CSV snapshot. - - The display row-limit is a rendering cap, not part of what the user asked - to see, so the export carries every filtered row - sorted the same way the - table is. The file is overwritten atomically each time (tmp + os.replace): - it is a snapshot of "the connections as they are now", not an append log - like the stats CSV. - """ - now = self.engine.now_ref() - snapshot = self.engine.connections_snapshot(limit=None) - # The export follows the view: what you exported and what you were looking - # at have to be the same set, or the file quietly disagrees with the screen - # that produced it. - if self.scoped_view(): - snapshot = [c for c in snapshot if c.get("scoped")] - rows = filter_sort_connections( - snapshot, self.conn_query, - self.conn_sort["col"], self.conn_sort["reverse"], - now=now, proc_map=self.proc_map, limit=0) - path = CONNECTIONS_CSV_FILE - tmp = path + ".tmp" - try: - with open(tmp, "w", newline="", encoding="utf-8") as f: - writer = csv.writer(f) - writer.writerow(self.CONN_CSV_HEADER) - for c in rows: - last = c.get("last", now) - packets = c.get("packets", 0) or 0 - writer.writerow([ - connection_proc(c, self.proc_map) or "?", - c.get("pid") or "", - c.get("proto", "IP"), c.get("remote_ip", ""), - c.get("remote_port", ""), c.get("local_port", ""), - packets, "yes" if c.get("scoped") else "no", - c.get("dropped", 0), - c.get("sent_in", 0), c.get("sent_out", 0), c.get("sent", 0), - c.get("bytes_in", 0), c.get("bytes_out", 0), c.get("bytes", 0), - avg_packet_bytes(c), - f"{max(0.0, last - c.get('first', now)):.1f}", - f"{max(0.0, now - last):.1f}"]) - os.replace(tmp, path) - self.log(f"{T('log.conns_saved_to')} {os.path.basename(path)} ({len(rows)})") - except Exception as e: - # Clean up the half-written temp file, the way jsonfile.write_json - # already does. Without this a failed export left a `.csv.tmp` next to - # the real file for the user to find and wonder about - and the next - # export silently overwrote it, so the litter was never even stable. - # A failure here must leave the previous export untouched and nothing - # else behind. - try: - if os.path.exists(tmp): - os.remove(tmp) - except OSError as _exc: - crashlog.note(_exc, "gui.app") - self.log(f"{T('log.csv_error')}: {e}") + csv_export.export_connections(self) def mark_bug(self): if not self.running: @@ -1744,6 +1612,10 @@ def on_close(self): def _report_storage_problems(self): """A profile file that vanished or broke must SAY so, not just be gone.""" + # No reset, unlike the stores below: this list is filled once, before the + # window exists, and _check_environment reads it once. + for problem in self._data_problems: + self.log(f"{T('log.data_files_problem')}: {problem}") for store, key in ((self.profiles, "log.profiles_problem"), (self.ui, "log.ui_state_problem")): problem = getattr(store, "problem", None) diff --git a/beantester/gui/csv_export.py b/beantester/gui/csv_export.py new file mode 100644 index 0000000..5dfb5f8 --- /dev/null +++ b/beantester/gui/csv_export.py @@ -0,0 +1,167 @@ +"""The two CSV exports: the appended session stats and the connection snapshot. + +Carved out of ``gui/app.py`` when it sat at the size ceiling (convention 32b says +split rather than raise the number). They still read the App - the engine snapshot, +the current view, the log - so they take it as an argument instead of pretending to +be independent, and ``App.export_csv`` / ``App.export_connections_csv`` stay as the +names every caller already uses. + +The column tables live here too, and they are the source the README guard reads. +""" +import csv +import os +import time + +from .. import crashlog +from ..i18n import T +from ..paths import CONNECTIONS_CSV_FILE, CSV_FILE +from ..views import avg_packet_bytes, connection_proc, filter_sort_connections + +# Internal stat keys are engine-speak ("seen"); a CSV is read by people and +# by spreadsheets, so it gets column names that mean something. +CSV_COLUMNS = {"seen": "packets_seen", "scoped_seen": "packets_in_scope", + "drop_loss": "dropped_loss", + "drop_overflow": "dropped_overflow", "drop_syn": "dropped_syn", + "drop_mtu": "dropped_mtu", "drop_nat": "dropped_nat", + "drop_rst": "dropped_rst", "rst_reset": "connections_reset", + "drop_lan": "dropped_lan", + "drop_block": "dropped_block", + "drop_flap": "dropped_link_outage", "drop_rate": "dropped_rate_limit", + "drop_shutdown": "dropped_at_stop", + "drop_send": "dropped_send_failed", + "queue": "queue_len", + "peak_queue": "queue_peak", + # The stats CSV deliberately does NOT follow the "show only the + # targeted traffic" preference: it is an APPEND log, and a file + # whose columns mean one thing in some rows and another in the + # rest is worse than useless for the spreadsheet it exists for. + # It gains both totals instead, so the reader can do the + # narrowing themselves and see which is which. + "bytes_in_scoped": "delivered_in_scope_bytes_down", + "bytes_out_scoped": "delivered_in_scope_bytes_up"} + +# Columns that come from the SESSION rather than the counters: they say which +# world the counters were measured in. The comment above explains why this +# file must not follow the view preference - a column meaning one thing in +# some rows and another in the rest is useless to the spreadsheet it exists +# for. Capture narrowing is the STRONGER version of that problem and had no +# column at all: it does not pick between two totals, it changes `seen` +# itself, so a narrowed row and a wide row were indistinguishable while +# counting completely different traffic. The CLI has carried the same fact in +# its JSON summary (`capture_narrowed`) since narrowing shipped, and the repro +# report carries the whole `session_info` - only this file could not say. +# Keyed by session_info() key, like CSV_COLUMNS is keyed by counter key. +CSV_SESSION_COLUMNS = {"narrowed": "capture_narrowed"} + + +def session_value(key, info): + """One session cell. Booleans as yes/no in English, like `impaired` in the + connections export - a CSV is read by scripts and spreadsheets, so it does + not follow the interface language.""" + value = info.get(key) + return ("yes" if value else "no") if isinstance(value, bool) else value + + +def export_stats(app): + snap = app.engine.stats_snapshot() + info = app.engine.session_info() + session_cells = [session_value(k, info) + for k in CSV_SESSION_COLUMNS] + header = ["time", *CSV_SESSION_COLUMNS.values(), + *(CSV_COLUMNS.get(k, k) for k in snap)] + try: + write_header = not os.path.exists(CSV_FILE) + if not write_header: + with open(CSV_FILE, newline="", encoding="utf-8") as f: + existing = next(csv.reader(f), []) + if existing != header: + # the stat columns changed between versions: appending would + # silently misalign rows against the old header + backup = CSV_FILE[:-4] + time.strftime(".%Y%m%d-%H%M%S.csv") + os.replace(CSV_FILE, backup) + app.log(f"{T('log.csv_rotated')} {os.path.basename(backup)}") + write_header = True + with open(CSV_FILE, "a", newline="", encoding="utf-8") as f: + writer = csv.writer(f) + if write_header: + writer.writerow(header) + writer.writerow([time.strftime("%Y-%m-%d %H:%M:%S"), + *session_cells, *snap.values()]) + app.log(f"{T('log.stats_saved_to')} {os.path.basename(CSV_FILE)}") + except Exception as e: + app.log(f"{T('log.csv_error')}: {e}") + +# Mirrors the table's columns so the export is the table, on disk. Raw bytes, +# not KB: a CSV is read by a spreadsheet or a script, where exact, summable +# integers beat the one-decimal KB the table shows for people. `impaired` is +# "yes"/"no" in English, like the headers - the CSV is language-independent. +# delivered_* is what reached the application; captured_* is what the tool +# saw offered. The old download_bytes/upload_bytes/total_bytes held CAPTURED +# under names every other surface uses for delivered, so they are renamed +# rather than reused - a column that quietly changes meaning is worse than one +# that disappears. +CONN_CSV_HEADER = ["process", "pid", "proto", "remote_ip", "remote_port", + "local_port", "packets", "impaired", "dropped", + "delivered_down_bytes", "delivered_up_bytes", + "delivered_total_bytes", + "captured_down_bytes", "captured_up_bytes", + "captured_total_bytes", + "avg_bytes", "duration_s", "idle_s"] + + +def export_connections(app): + """Write the CURRENT connection view (search + sort) to a CSV snapshot. + + The display row-limit is a rendering cap, not part of what the user asked + to see, so the export carries every filtered row - sorted the same way the + table is. The file is overwritten atomically each time (tmp + os.replace): + it is a snapshot of "the connections as they are now", not an append log + like the stats CSV. + """ + now = app.engine.now_ref() + snapshot = app.engine.connections_snapshot(limit=None) + # The export follows the view: what you exported and what you were looking + # at have to be the same set, or the file quietly disagrees with the screen + # that produced it. + if app.scoped_view(): + snapshot = [c for c in snapshot if c.get("scoped")] + rows = filter_sort_connections( + snapshot, app.conn_query, + app.conn_sort["col"], app.conn_sort["reverse"], + now=now, proc_map=app.proc_map, limit=0) + path = CONNECTIONS_CSV_FILE + tmp = path + ".tmp" + try: + with open(tmp, "w", newline="", encoding="utf-8") as f: + writer = csv.writer(f) + writer.writerow(CONN_CSV_HEADER) + for c in rows: + last = c.get("last", now) + packets = c.get("packets", 0) or 0 + writer.writerow([ + connection_proc(c, app.proc_map) or "?", + c.get("pid") or "", + c.get("proto", "IP"), c.get("remote_ip", ""), + c.get("remote_port", ""), c.get("local_port", ""), + packets, "yes" if c.get("scoped") else "no", + c.get("dropped", 0), + c.get("sent_in", 0), c.get("sent_out", 0), c.get("sent", 0), + c.get("bytes_in", 0), c.get("bytes_out", 0), c.get("bytes", 0), + avg_packet_bytes(c), + f"{max(0.0, last - c.get('first', now)):.1f}", + f"{max(0.0, now - last):.1f}"]) + os.replace(tmp, path) + app.log(f"{T('log.conns_saved_to')} {os.path.basename(path)} ({len(rows)})") + except Exception as e: + # Clean up the half-written temp file, the way jsonfile.write_json + # already does. Without this a failed export left a `.csv.tmp` next to + # the real file for the user to find and wonder about - and the next + # export silently overwrote it, so the litter was never even stable. + # A failure here must leave the previous export untouched and nothing + # else behind. + try: + if os.path.exists(tmp): + os.remove(tmp) + except OSError as _exc: + crashlog.note(_exc, "gui.app") + app.log(f"{T('log.csv_error')}: {e}") diff --git a/beantester/paths.py b/beantester/paths.py index d284763..5ddd33a 100644 --- a/beantester/paths.py +++ b/beantester/paths.py @@ -3,31 +3,166 @@ The tool can run in three layouts: * from sources - resources live in the project root (parent of the package), * as a PyInstaller exe - bundled resources live in ``sys._MEIPASS``, while files - the user cares about (profiles, CSV) are written next to the executable, + the user cares about (profiles, CSV) are written into the user's data directory, * installed package - resources may live inside the package directory. + +The user files used to sit next to the executable. They no longer do, and the +reason is package managers - see ``user_data_dir``. """ import os +import shutil import sys PACKAGE_DIR = os.path.dirname(os.path.abspath(__file__)) PROJECT_ROOT = os.path.dirname(PACKAGE_DIR) +# An override for a genuinely portable copy (a stick, a shared folder). Everything +# else is derived, so this is the ONLY knob. +DATA_DIR_ENV = "BEAN_DATA_DIR" + +# The files a user would miss. The NAMES are unchanged - both READMEs document +# them and a repro command may name them - only the directory moved. +PROFILE_NAME = "bean_network_tester_profiles.json" +STATS_CSV_NAME = "bean_network_tester_stats.csv" +CONNECTIONS_CSV_NAME = "bean_network_tester_connections.csv" +UI_STATE_NAME = "bean_network_tester_ui.json" + +# What migration carries. Derived from the names above rather than repeated: a new +# user file that is added below and forgotten here would simply never be adopted, +# and nothing would say so. +USER_FILE_NAMES = (PROFILE_NAME, STATS_CSV_NAME, CONNECTIONS_CSV_NAME, UI_STATE_NAME) + def is_frozen(): """True when running from a PyInstaller bundle.""" return bool(getattr(sys, "frozen", False)) -def app_dir(): - """Directory for user-facing output files (profiles, CSV stats). +def executable_dir(): + """Directory holding the running executable (meaningful when frozen).""" + return os.path.dirname(os.path.abspath(sys.executable)) + + +def _local_app_data(): + """``%LOCALAPPDATA%``, with a fallback for a profile that does not set it.""" + base = os.environ.get("LOCALAPPDATA") or "" + if not base: + base = os.path.join(os.path.expanduser("~"), "AppData", "Local") + return base + + +def user_data_dir(): + """Directory for the files the user would miss: profiles, window state, CSV. + + A frozen build writes into ``%LOCALAPPDATA%\\``, NOT next to the + executable, because a package manager owns the install directory: + + * WinGet records the extracted archive's top-level directory as one entry and + an upgrade removes every recorded entry before installing the new ones - + ``remove_all`` on that directory, with no diffing and no hook a manifest + could use. Our zip has exactly one top-level directory, so everything + written beside the exe is gone on the next ``winget upgrade``. + * Chocolatey keeps files it never installed, but its package folder grants + plain users read and execute only, so a non-elevated run cannot save there + at all. + + The answer deliberately does NOT depend on whether the executable's directory + happens to be writable. Probing for that would make the location depend on + ELEVATION - the GUI elevates itself, ``--simulate`` does not - and the same + install would then keep two sets of profiles without saying so. - Next to the executable when frozen (a onefile exe unpacks itself into a - temporary directory, which would silently swallow saved profiles), - otherwise the project root. + ``BEAN_DATA_DIR`` overrides everything. Running from sources is unchanged. + """ + override = os.environ.get(DATA_DIR_ENV, "").strip() + if override: + return os.path.abspath(override) + if not is_frozen(): + return PROJECT_ROOT + # Imported here rather than at module scope: appinfo reads VERSION.txt through + # resource_path (below), so a top-level import would run appinfo against a + # half-initialised paths module. + from .appinfo import TOOL_ID + return os.path.join(_local_app_data(), TOOL_ID) + + +def ensure_data_dir(): + """Create the data directory. Returns an error message, or None. + + It has to happen here because neither writer will do it: ``write_json`` + refuses to invent a directory on purpose (a typo in ``--save-config`` must not + become a silent success) and the CSV export is a plain ``open``. + """ + try: + os.makedirs(user_data_dir(), exist_ok=True) + except OSError as e: + return str(e) + return None + + +def _same_directory(a, b): + return os.path.normcase(os.path.abspath(a)) == os.path.normcase(os.path.abspath(b)) + + +def migrate_user_files(source=None, target=None): + """Adopt user files left beside a frozen executable. Returns problems, never raises. + + Two decisions worth keeping: + + * it COPIES rather than moves, so rolling back to an older build still finds + the files where that build looks for them; + * it only takes a file the target does NOT have, so a stale copy beside the + exe can never overwrite newer data. + + Nothing is parsed on the way: a corrupt file is copied as it is, and the store + that reads it quarantines it exactly as it would have done before the move. + """ + explicit = source is not None or target is not None + if not explicit and not is_frozen(): + return [] # from sources the two are the same directory + source = executable_dir() if source is None else source + target = user_data_dir() if target is None else target + if _same_directory(source, target): + return [] + + problems = [] + try: + os.makedirs(target, exist_ok=True) + except OSError as e: + return [str(e)] + + for name in USER_FILE_NAMES: + src = os.path.join(source, name) + dst = os.path.join(target, name) + # isfile(), not exists(): a DIRECTORY carrying one of these names is what + # Scoop's persist leaves behind, and it must be skipped rather than copied. + if os.path.exists(dst) or not os.path.isfile(src): + continue + tmp = dst + ".tmp" + try: + shutil.copyfile(src, tmp) + os.replace(tmp, dst) # atomic, so a half-copy is never readable + except OSError as e: + problems.append(f"{name}: {e}") + try: + if os.path.exists(tmp): + os.remove(tmp) + except OSError as _exc: + # The leftover is harmless, the silence would not be (convention 30). + # Imported here because crashlog imports this module. + from . import crashlog + crashlog.note(_exc, "paths") + return problems + + +def prepare_user_data(): + """Make the data directory usable. Returns a list of problems (empty = fine). + + One call for the two things that must happen before any store reads a file. """ - if is_frozen(): - return os.path.dirname(os.path.abspath(sys.executable)) - return PROJECT_ROOT + error = ensure_data_dir() + if error: + return [error] + return migrate_user_files() def _resource_bases(): @@ -68,10 +203,10 @@ def scenarios_dir(): return os.path.join(PROJECT_ROOT, "scenarios") -PROFILE_FILE = os.path.join(app_dir(), "bean_network_tester_profiles.json") -CSV_FILE = os.path.join(app_dir(), "bean_network_tester_stats.csv") +PROFILE_FILE = os.path.join(user_data_dir(), PROFILE_NAME) +CSV_FILE = os.path.join(user_data_dir(), STATS_CSV_NAME) # Snapshot of the connection table (overwritten each export, unlike the appended # stats CSV): the user asks for "the connections as they are now". -CONNECTIONS_CSV_FILE = os.path.join(app_dir(), "bean_network_tester_connections.csv") +CONNECTIONS_CSV_FILE = os.path.join(user_data_dir(), CONNECTIONS_CSV_NAME) # Window geometry, active page, collapsed sections, table sorting, language... -UI_STATE_FILE = os.path.join(app_dir(), "bean_network_tester_ui.json") +UI_STATE_FILE = os.path.join(user_data_dir(), UI_STATE_NAME) diff --git a/lang/en.json b/lang/en.json index 4d04a9d..36c6e82 100644 --- a/lang/en.json +++ b/lang/en.json @@ -254,6 +254,7 @@ "log.copied": "Copied to clipboard", "log.csv_error": "CSV save error", "log.csv_rotated": "CSV column set changed - previous statistics moved to", + "log.data_files_problem": "Problem with the folder your files are saved in", "log.dest_frozen_while_narrowed": "Destination unchanged: with the driver filter narrowed, the destination is fixed for the session (the driver's filter is set when the capture opens). Stop and start again to change it.", "log.dest_set": "Destination set from the connection table", "log.donate_opened": "Support page opened", diff --git a/lang/pl.json b/lang/pl.json index 4bc5625..338fc0c 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -254,6 +254,7 @@ "log.copied": "Skopiowano do schowka", "log.csv_error": "Błąd zapisu CSV", "log.csv_rotated": "Zestaw kolumn CSV się zmienił - poprzednie statystyki przeniesiono do", + "log.data_files_problem": "Problem z folderem, w którym zapisują się Twoje pliki", "log.dest_frozen_while_narrowed": "Cel bez zmian: przy zawężonym filtrze sterownika cel jest ustalony na całą sesję (filtr sterownika powstaje przy otwarciu przechwytu). Zatrzymaj i uruchom ponownie, żeby go zmienić.", "log.dest_set": "Cel ustawiony z tabeli połączeń", "log.donate_opened": "Otwarto stronę wsparcia", diff --git a/smoke_gui.py b/smoke_gui.py index fc00549..aed7955 100644 --- a/smoke_gui.py +++ b/smoke_gui.py @@ -170,7 +170,7 @@ def leaked_keys(root): # -- CSV export: stale header rotates the old file, never misaligns rows ------ import csv as _csv # noqa: E402 import tempfile as _tempfile # noqa: E402 -import beantester.gui.app as _appmod # noqa: E402 +import beantester.gui.csv_export as _appmod # noqa: E402 _tmpdir = _tempfile.mkdtemp() _appmod.CSV_FILE = os.path.join(_tmpdir, "stats.csv") app._export_csv() @@ -178,7 +178,7 @@ def leaked_keys(root): with open(_appmod.CSV_FILE, newline="", encoding="utf-8") as _f: _rows = list(_csv.reader(_f)) # `capture_narrowed` sits between the timestamp and the counters: it records which -# world each row's numbers were measured in (App.CSV_SESSION_COLUMNS). +# world each row's numbers were measured in (csv_export.CSV_SESSION_COLUMNS). _SCOPE_HEAD = ["time", "capture_narrowed", "packets_seen"] check("GUI: CSV append keeps a single current header", len(_rows) == 3 and _rows[0][:3] == _SCOPE_HEAD, f"({_rows[:1]})") diff --git a/tests/conftest.py b/tests/conftest.py index 0d7a203..3413470 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -25,11 +25,11 @@ def _crash_log_outside_the_repo(tmp_path_factory): the sources on every run: git-ignored, so nothing ever showed it, and indistinguishable at a glance from a crash the developer actually hit. - ``tests/test_crashlog.py`` points ``app_dir`` at its own per-test directory on - top of this; a function-scoped monkeypatch wins over a session fixture, so the - two do not fight. + ``tests/test_crashlog.py`` points ``user_data_dir`` at its own per-test + directory on top of this; a function-scoped monkeypatch wins over a session + fixture, so the two do not fight. """ - crashlog.app_dir = lambda: str(tmp_path_factory.mktemp("crashlog")) + crashlog.user_data_dir = lambda: str(tmp_path_factory.mktemp("crashlog")) yield diff --git a/tests/test_code_shape.py b/tests/test_code_shape.py index 1b869a6..2793b82 100644 --- a/tests/test_code_shape.py +++ b/tests/test_code_shape.py @@ -54,7 +54,12 @@ # there for a week and were found only because somebody printed the numbers. That is # the same defect the crowd counts below exist to catch, one level up. FUNCTION_CEILING = 133 # beantester/cli.py::_run_session -FILE_CEILING = 1287 # beantester/gui/app.py +# Lowered 2026-08-12 from 1287, the same routine door: moving the user files out of +# the install directory needed three lines in `app.py`, which was pinned to the +# ceiling exactly, so the two CSV exports moved to `gui/csv_export.py` instead of the +# number moving up. The crowd band below was re-measured after the drop (`engine.py` +# is 779, still clear of it) - lowering a ceiling tightens that band too. +FILE_CEILING = 1202 # beantester/gui/app.py # 🔴 THE SECOND KNOB. A ceiling on the worst single item sees one thing growing # to a record and is blind to everything creeping upward together: five files at diff --git a/tests/test_conns_export.py b/tests/test_conns_export.py index 910b2fc..0026e34 100644 --- a/tests/test_conns_export.py +++ b/tests/test_conns_export.py @@ -13,7 +13,7 @@ def test_export_connections_csv_writes_the_current_view(): run_gui(''' import os, tempfile, csv - import beantester.gui.app as m + import beantester.gui.csv_export as m path = os.path.join(tempfile.mkdtemp(), "conns.csv") m.CONNECTIONS_CSV_FILE = path @@ -76,7 +76,7 @@ def test_a_failed_connections_export_leaves_no_tmp_file_behind(): """ run_gui(''' import os, tempfile - import beantester.gui.app as m + import beantester.gui.csv_export as m path = os.path.join(tempfile.mkdtemp(), "conns.csv") m.CONNECTIONS_CSV_FILE = path @@ -103,7 +103,7 @@ def test_export_connections_csv_writes_a_portless_row_with_empty_port_cells(): out EMPTY, not "None" and not shifted - a misaligned row here is silent.""" run_gui(''' import os, tempfile, csv - import beantester.gui.app as m + import beantester.gui.csv_export as m path = os.path.join(tempfile.mkdtemp(), "conns.csv") m.CONNECTIONS_CSV_FILE = path @@ -131,7 +131,7 @@ def test_export_connections_csv_writes_a_portless_row_with_empty_port_cells(): def test_export_connections_csv_honours_the_search(): run_gui(''' import os, tempfile, csv - import beantester.gui.app as m + import beantester.gui.csv_export as m path = os.path.join(tempfile.mkdtemp(), "conns.csv") m.CONNECTIONS_CSV_FILE = path @@ -162,7 +162,7 @@ def test_export_connections_csv_avg_matches_the_table_rounding(): """ run_gui(''' import os, tempfile, csv - import beantester.gui.app as m + import beantester.gui.csv_export as m from beantester.views import avg_packet_bytes path = os.path.join(tempfile.mkdtemp(), "conns.csv") m.CONNECTIONS_CSV_FILE = path @@ -186,14 +186,14 @@ def test_export_connections_csv_avg_matches_the_table_rounding(): def test_export_csv_stats_appends_then_rotates_on_a_column_change(): run_gui(''' import os, tempfile, csv - import beantester.gui.app as m + import beantester.gui.csv_export as m path = os.path.join(tempfile.mkdtemp(), "stats.csv") m.CSV_FILE = path # first two exports share a column set: header once, then two data rows. # `capture_narrowed` comes from the SESSION, not the counters, and sits # between the timestamp and them - it says which world the row's numbers - # were measured in (see App.CSV_SESSION_COLUMNS). + # were measured in (see csv_export.CSV_SESSION_COLUMNS). app.engine.stats_snapshot = lambda: {"seen": 100, "drop_loss": 5, "queue": 2} app.export_csv() app.export_csv() diff --git a/tests/test_crashlog.py b/tests/test_crashlog.py index 8444a9b..8f05484 100644 --- a/tests/test_crashlog.py +++ b/tests/test_crashlog.py @@ -29,7 +29,7 @@ @pytest.fixture(autouse=True) def isolated(tmp_path, monkeypatch): """Point the logger at a temp dir; never touch the real crash folder.""" - monkeypatch.setattr(crashlog, "app_dir", lambda: str(tmp_path)) + monkeypatch.setattr(crashlog, "user_data_dir", lambda: str(tmp_path)) crashlog.reset() crashlog.set_enabled(True) crashlog.set_context_provider(None) diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 7b8c4ab..675f60e 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -71,6 +71,16 @@ "new": ' return bool(key == "duration" and getattr(self.app, "running", False))', "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + # The whole point of moving the user files: a package manager owns the + # install directory and wipes it on upgrade. This is the old behaviour + # put back, which is also what any writability probe would degrade into. + "label": "paths: a frozen build writes user files next to the executable again", + "file": "beantester/paths.py", + "old": " return os.path.join(_local_app_data(), TOOL_ID)", + "new": " return executable_dir()", + "test": "test_a_frozen_build_keeps_no_user_file_next_to_the_executable", + }, { # The display's source order. Reversed, a connection row names whatever a # snapshot taken a few times a second last saw, while the gate is judging diff --git a/tests/test_readme_guards.py b/tests/test_readme_guards.py index 3c0bf5f..3d7b068 100644 --- a/tests/test_readme_guards.py +++ b/tests/test_readme_guards.py @@ -112,13 +112,13 @@ def test_both_readmes_document_every_csv_column(): the connections export deliberately does NOT reuse the table's labels (``impaired`` for "impaired?", ``delivered_down_bytes`` for "down[KB]"). """ - from beantester.gui.app import App - names = set(App.CONN_CSV_HEADER) - names |= {App.CSV_COLUMNS.get(k, k) for k in App.CSV_COLUMNS} + from beantester.gui import csv_export + names = set(csv_export.CONN_CSV_HEADER) + names |= {csv_export.CSV_COLUMNS.get(k, k) for k in csv_export.CSV_COLUMNS} # Session columns are part of the same header and just as undocumentable by # inspection - `capture_narrowed` decides what `packets_seen` next to it even # counted. - names |= set(App.CSV_SESSION_COLUMNS.values()) + names |= set(csv_export.CSV_SESSION_COLUMNS.values()) for readme in READMES: text = _read(readme) missing = sorted(n for n in names if n not in text) diff --git a/tests/test_release_fixes.py b/tests/test_release_fixes.py index a1d8746..a2f95a0 100644 --- a/tests/test_release_fixes.py +++ b/tests/test_release_fixes.py @@ -336,9 +336,9 @@ def uniform(a, b): # -- the CSV column names are meant for humans ---------------------------------- # def test_the_csv_has_readable_column_names(): - """Read as text: importing gui.app here would need tkinter, which the CLI does not.""" + """Read as text: importing the gui package here would need tkinter, which the CLI does not.""" source = open(os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), - "beantester", "gui", "app.py"), encoding="utf-8").read() + "beantester", "gui", "csv_export.py"), encoding="utf-8").read() check("'seen' is not a column name", '"seen": "packets_seen"' in source) check("link outages have a name too", '"drop_flap": "dropped_link_outage"' in source) diff --git a/tests/test_user_data_location.py b/tests/test_user_data_location.py new file mode 100644 index 0000000..0d91aaf --- /dev/null +++ b/tests/test_user_data_location.py @@ -0,0 +1,194 @@ +"""Where a frozen build keeps the user's files, and how it adopts the old ones. + +This guards a behaviour that had NO test at all until the files moved: their +location. ``tests/user_files.py`` redirects every store to a temp directory, which +is right for the rest of the suite and makes it blind to exactly this question - +so the location is pinned here, and only here. + +What is being protected: a package manager owns the install directory. WinGet's +upgrade removes the extracted archive's directory recursively before installing +the new one, so anything written beside the executable is gone on the next +upgrade. The four files therefore live in the user's data directory, and an +older build's files are copied into it once. + +The frozen cases run in a SUBPROCESS on purpose: ``paths`` decides the four +constants at import time, so faking ``sys.frozen`` in this process would either +do nothing or leave the rest of the suite reading a fake location. +""" +import json +import os +import subprocess +import sys + +from fakes import ROOT, check + +from beantester import paths + +CHILD = """ +import json, os, sys +sys.frozen = True # what PyInstaller sets +sys.executable = os.path.join(sys.argv[1], "BeanNetworkTester.exe") +sys.path.insert(0, sys.argv[2]) +from beantester import paths +print(json.dumps({ + "data_dir": paths.user_data_dir(), + "profiles": paths.PROFILE_FILE, + "stats": paths.CSV_FILE, + "connections": paths.CONNECTIONS_CSV_FILE, + "ui": paths.UI_STATE_FILE, + "adopted": paths.prepare_user_data(), +})) +""" + + +def _frozen_paths(tmp_path, exe_dir, **env): + """Import ``paths`` as a frozen build would, in a throwaway interpreter.""" + script = tmp_path / "child.py" + script.write_text(CHILD, encoding="utf-8") + environment = dict(os.environ) + environment.pop(paths.DATA_DIR_ENV, None) + environment["LOCALAPPDATA"] = str(tmp_path / "localappdata") + environment.update(env) + out = subprocess.run([sys.executable, str(script), str(exe_dir), ROOT], + capture_output=True, text=True, env=environment, + timeout=120) + check("the frozen child ran", out.returncode == 0, f"({out.stderr[-400:]})") + return json.loads(out.stdout) + + +# -- where the files live ------------------------------------------------------ # +def test_a_frozen_build_keeps_no_user_file_next_to_the_executable(tmp_path): + exe_dir = tmp_path / "install" / "BeanNetworkTester" + exe_dir.mkdir(parents=True) + got = _frozen_paths(tmp_path, exe_dir) + + inside = [name for name in ("profiles", "stats", "connections", "ui") + if os.path.dirname(got[name]) == str(exe_dir)] + check("no user file is written next to the exe", not inside, f"({inside})") + check("the data directory is under LOCALAPPDATA", + got["data_dir"] == str(tmp_path / "localappdata" / "BeanNetworkTester"), + f"({got['data_dir']})") + + +def test_every_user_file_lands_in_the_data_directory(tmp_path): + exe_dir = tmp_path / "install" + exe_dir.mkdir() + got = _frozen_paths(tmp_path, exe_dir) + + stray = [name for name in ("profiles", "stats", "connections", "ui") + if os.path.dirname(got[name]) != got["data_dir"]] + check("all four files share the data directory", not stray, f"({stray})") + + +def test_the_data_directory_can_be_overridden_for_a_portable_copy(tmp_path): + exe_dir = tmp_path / "install" + exe_dir.mkdir() + portable = tmp_path / "stick" / "data" + got = _frozen_paths(tmp_path, exe_dir, **{paths.DATA_DIR_ENV: str(portable)}) + + check("BEAN_DATA_DIR wins", got["data_dir"] == str(portable), f"({got['data_dir']})") + check("the directory is created for the first write", + os.path.isdir(portable), "(missing)") + + +def test_running_from_sources_still_uses_the_project_root(): + check("sources are unchanged", paths.user_data_dir() == paths.PROJECT_ROOT, + f"({paths.user_data_dir()})") + check("and nothing is migrated there", paths.migrate_user_files() == [], + "(a source tree must never be written to by a migration)") + + +# -- adopting the files an older build left behind ----------------------------- # +def _dirs(tmp_path): + source, target = tmp_path / "old", tmp_path / "new" + source.mkdir() + target.mkdir() + return source, target + + +def test_a_file_the_target_does_not_have_is_adopted(tmp_path): + source, target = _dirs(tmp_path) + (source / paths.PROFILE_NAME).write_text('{"slow": {}}', encoding="utf-8") + + problems = paths.migrate_user_files(str(source), str(target)) + + check("no problems", problems == [], f"({problems})") + check("the profile arrived", + (target / paths.PROFILE_NAME).read_text(encoding="utf-8") == '{"slow": {}}') + check("the original is left in place (a rollback still finds it)", + (source / paths.PROFILE_NAME).exists()) + + +def test_a_stale_copy_never_overwrites_newer_data(tmp_path): + source, target = _dirs(tmp_path) + (source / paths.PROFILE_NAME).write_text("OLD", encoding="utf-8") + (target / paths.PROFILE_NAME).write_text("NEW", encoding="utf-8") + + paths.migrate_user_files(str(source), str(target)) + + check("the file already in the data directory wins", + (target / paths.PROFILE_NAME).read_text(encoding="utf-8") == "NEW") + + +def test_a_corrupt_file_is_copied_verbatim_rather_than_parsed(tmp_path): + source, target = _dirs(tmp_path) + (source / paths.UI_STATE_NAME).write_text("", encoding="utf-8") + + problems = paths.migrate_user_files(str(source), str(target)) + + check("a zero-byte file is not a migration problem", problems == [], f"({problems})") + check("it arrives as it was, for the store to quarantine", + (target / paths.UI_STATE_NAME).read_text(encoding="utf-8") == "") + + +def test_a_directory_carrying_a_user_file_name_is_skipped(tmp_path): + # What Scoop's persist leaves behind when it persists a file that does not + # exist yet. Copying it would raise, and raising here would break startup. + source, target = _dirs(tmp_path) + (source / paths.UI_STATE_NAME).mkdir() + + problems = paths.migrate_user_files(str(source), str(target)) + + check("no problem is reported", problems == [], f"({problems})") + check("and nothing was created in the data directory", + not (target / paths.UI_STATE_NAME).exists()) + + +def test_migrating_a_directory_onto_itself_does_nothing(tmp_path): + source, _ = _dirs(tmp_path) + (source / paths.PROFILE_NAME).write_text("{}", encoding="utf-8") + + problems = paths.migrate_user_files(str(source), str(source).upper() + if os.name == "nt" else str(source)) + + check("same directory = no work", problems == [], f"({problems})") + + +def test_a_failed_copy_is_reported_and_leaves_no_half_written_file(tmp_path, monkeypatch): + source, target = _dirs(tmp_path) + (source / paths.PROFILE_NAME).write_text("{}", encoding="utf-8") + + def boom(src, dst): + with open(dst, "w", encoding="utf-8") as f: + f.write("half") # the temp file exists when it fails + raise OSError("disk full") + + monkeypatch.setattr(paths.shutil, "copyfile", boom) + problems = paths.migrate_user_files(str(source), str(target)) + + check("the failure is reported, not swallowed", len(problems) == 1, f"({problems})") + check("the file name is in the message", paths.PROFILE_NAME in problems[0], + f"({problems})") + leftovers = sorted(p.name for p in target.iterdir()) + check("no half-written file and no .tmp is left", leftovers == [], f"({leftovers})") + + +def test_an_unusable_data_directory_does_not_stop_the_program(tmp_path, monkeypatch): + blocked = tmp_path / "not-a-directory" + blocked.write_text("I am a file", encoding="utf-8") + monkeypatch.setattr(paths, "user_data_dir", lambda: str(blocked)) + + problems = paths.prepare_user_data() + + check("startup gets a problem to log instead of an exception", + len(problems) == 1 and problems[0], f"({problems})") diff --git a/tests/test_view_scope.py b/tests/test_view_scope.py index f88ee4d..633050d 100644 --- a/tests/test_view_scope.py +++ b/tests/test_view_scope.py @@ -687,7 +687,7 @@ def test_every_wording_table_is_complete_and_every_key_has_text(): def test_the_stats_csv_carries_both_totals_and_never_narrows(): """It is an append log: rows written under different preferences must compare.""" run_gui(""" - cols = app.CSV_COLUMNS + from beantester.gui.csv_export import CSV_COLUMNS as cols assert "bytes_in_scoped" in cols and "bytes_out_scoped" in cols, cols assert cols["seen"] == "packets_seen", cols["seen"] assert cols["scoped_seen"] == "packets_in_scope", cols["scoped_seen"] @@ -708,10 +708,10 @@ def test_the_stats_csv_records_which_world_each_row_was_measured_in(): out = run_gui(""" import csv, os, tempfile - from beantester.gui import app as app_mod + from beantester.gui import csv_export as csv_mod path = os.path.join(tempfile.mkdtemp(), "stats.csv") - app_mod.CSV_FILE = path + csv_mod.CSV_FILE = path app.engine._narrowed = False app.export_csv() diff --git a/tests/user_files.py b/tests/user_files.py index 0e0efa6..660078e 100644 --- a/tests/user_files.py +++ b/tests/user_files.py @@ -31,5 +31,5 @@ def redirect_to_temp(directory=None): os.path.join(directory, "ui.json"),) profiles.ProfileStore.__init__.__defaults__ = ( os.path.join(directory, "profiles.json"),) - crashlog.app_dir = lambda: directory + crashlog.user_data_dir = lambda: directory return directory From cb46e11589b3009b9849fb86f350346633f6d296 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 17:22:06 +0200 Subject: [PATCH 2/7] docs(paths): say where the user files are, in the program and in both READMEs The prose that read "next to the executable" was true until the previous commit moved the files, and nothing guards prose (convention 5), so it is swept in the same chunk as the behaviour. - both CSV exports log the WHOLE path instead of the file name: the name was an answer only while the file landed next to the executable, and nothing else on screen names the directory - module docstrings: jsonfile, gui/profiles, gui/ui_state (which used the old location to justify why these files are hand-edited) - both READMEs: the CSV section now names the folder, says why the files left the program folder, and documents BEAN_DATA_DIR; gui/csv_export.py joins the module list Guard: tests/test_conns_export.py::test_both_exports_tell_the_user_the_whole_path, proved by mutation and registered - putting the basename back reddens it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 10 +++++----- README.md | 9 +++++++-- README.pl.md | 8 ++++++-- beantester/gui/csv_export.py | 6 ++++-- beantester/gui/profiles.py | 2 +- beantester/gui/ui_state.py | 4 ++-- beantester/jsonfile.py | 9 +++++---- tests/test_conns_export.py | 32 ++++++++++++++++++++++++++++++++ tests/test_mutation_registry.py | 9 +++++++++ 9 files changed, 71 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ea0286..5063619 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,11 +54,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ### Changed - **Your profiles, window state and CSV exports now live in your own user folder** - (`%LOCALAPPDATA%\BeanNetworkTester`) instead of the program folder. The first time you start - this version, files from an earlier one are copied over and the originals are left where they - were, so going back to an older build still finds them. The reason is updates: a package - manager owns the program folder and replaces it, which would take your saved profiles with it. - Set `BEAN_DATA_DIR` to any folder to keep everything together instead, for example on a stick. + (`%LOCALAPPDATA%\BeanNetworkTester`) instead of the program folder. Files from an earlier + version are copied over on first start, and the originals stay where they were, so going back + to an older build still finds them. The reason is updates: a package manager owns the program + folder and replaces it, which would take your profiles with it. Both CSV exports now log the + whole path. Set `BEAN_DATA_DIR` to keep everything somewhere else, for example on a stick. ### Docs - **Four more guides on the website: no internet, timed scenarios, game lag and chaos testing.** diff --git a/README.md b/README.md index 3eef99c..05c904b 100644 --- a/README.md +++ b/README.md @@ -829,8 +829,13 @@ meant delivered: a row could read 5 MB received while its application got 0.4 MB ## CSV exports -Two buttons write two files, and they behave **differently on purpose**. Both land next to the -executable (or in the project root when running from source). +Two buttons write two files, and they behave **differently on purpose**. Both land in your own +folder, `%LOCALAPPDATA%\BeanNetworkTester`, together with your profiles and the window state (or +in the project root when running from source). The program writes the full path into the log +every time it saves one. They are kept out of the program folder so that updating the program - +by hand or through a package manager, which replaces that folder - cannot take your files with +it. Set `BEAN_DATA_DIR` to a folder of your choosing to keep everything somewhere else, for +example on the same stick as a portable copy. | | **Statistics** ("Export CSV", Statistics -> Live) | **Connections** ("Export connections CSV") | |---|---|---| diff --git a/README.pl.md b/README.pl.md index 4b0b9be..a85e22e 100644 --- a/README.pl.md +++ b/README.pl.md @@ -688,8 +688,12 @@ dostarczone: wiersz mógł pokazywać 5 MB odebranych, gdy aplikacja dostała 0, ## Eksporty CSV -Dwa przyciski zapisują dwa pliki i **celowo zachowują się inaczej**. Oba lądują obok pliku -wykonywalnego (albo w korzeniu projektu przy uruchomieniu ze źródeł). +Dwa przyciski zapisują dwa pliki i **celowo zachowują się inaczej**. Oba lądują w Twoim własnym +folderze, `%LOCALAPPDATA%\BeanNetworkTester`, razem z profilami i stanem okna (albo w korzeniu +projektu przy uruchomieniu ze źródeł). Program przy każdym zapisie wpisuje do logu pełną ścieżkę. +Leżą poza folderem programu po to, żeby aktualizacja - ręczna albo przez menedżer pakietów, który +ten folder podmienia - nie zabrała Twoich plików razem z nim. Ustaw `BEAN_DATA_DIR` na wybrany +folder, żeby trzymać wszystko gdzie indziej, na przykład na tym samym pendrivie co kopia programu. | | **Statystyki** („Eksportuj CSV", Statystyki → Na żywo) | **Połączenia** („Eksportuj połączenia CSV") | |---|---|---| diff --git a/beantester/gui/csv_export.py b/beantester/gui/csv_export.py index 5dfb5f8..c6e7f1d 100644 --- a/beantester/gui/csv_export.py +++ b/beantester/gui/csv_export.py @@ -87,7 +87,9 @@ def export_stats(app): writer.writerow(header) writer.writerow([time.strftime("%Y-%m-%d %H:%M:%S"), *session_cells, *snap.values()]) - app.log(f"{T('log.stats_saved_to')} {os.path.basename(CSV_FILE)}") + # The WHOLE path, not just the file name: these files no longer sit next to + # the executable, so the name alone would leave the user hunting for them. + app.log(f"{T('log.stats_saved_to')} {CSV_FILE}") except Exception as e: app.log(f"{T('log.csv_error')}: {e}") @@ -151,7 +153,7 @@ def export_connections(app): f"{max(0.0, last - c.get('first', now)):.1f}", f"{max(0.0, now - last):.1f}"]) os.replace(tmp, path) - app.log(f"{T('log.conns_saved_to')} {os.path.basename(path)} ({len(rows)})") + app.log(f"{T('log.conns_saved_to')} {path} ({len(rows)})") except Exception as e: # Clean up the half-written temp file, the way jsonfile.write_json # already does. Without this a failed export left a `.csv.tmp` next to diff --git a/beantester/gui/profiles.py b/beantester/gui/profiles.py index 30b330d..8a38ae7 100644 --- a/beantester/gui/profiles.py +++ b/beantester/gui/profiles.py @@ -1,4 +1,4 @@ -"""Persistence of user-defined profiles (JSON file next to the app). +"""Persistence of user-defined profiles (a JSON file in the user's data directory). A profile stores the link-characteristic fields a preset stores (see ``presets.settings_to_preset``) - the field registry decides which those are. diff --git a/beantester/gui/ui_state.py b/beantester/gui/ui_state.py index 7b5e9c4..7be9252 100644 --- a/beantester/gui/ui_state.py +++ b/beantester/gui/ui_state.py @@ -44,8 +44,8 @@ def _clean(data): """Drop values whose TYPE is not the one ``DEFAULTS`` promises. ``read_json`` guarantees the file is a dict and nothing beyond that. Inside - it, a hand edit - and these files are meant to be hand-edited, they live - next to the executable - can leave any value any shape. Three of them used + it, a hand edit - and these files are meant to be hand-edited, they sit in + a directory of the user's own - can leave any value any shape. Three of them used to stop the app from starting at all: a list under ``page`` (unhashable as a dict key), a list under ``conn_sort``, a string under ``event_sort``. The promise at the top of this module is the exact opposite, so it is now diff --git a/beantester/jsonfile.py b/beantester/jsonfile.py index 2fc1d9f..493b354 100644 --- a/beantester/jsonfile.py +++ b/beantester/jsonfile.py @@ -1,9 +1,10 @@ """Crash-safe JSON persistence for the tool's user files. -Profiles, window state and config files are edited by hand (they live next to -the executable on purpose), they are deleted, they are copied between machines -and - if the process dies mid-write - they get truncated. None of that may take -the app down, and none of it may destroy data silently: +Profiles, window state and config files are edited by hand (they live in one +directory of the user's own, see ``paths.user_data_dir``), they are deleted, they +are copied between machines and - if the process dies mid-write - they get +truncated. None of that may take the app down, and none of it may destroy data +silently: * **atomic writes** - the new content is written to a temporary file and then ``os.replace``d over the target, which is atomic on Windows and POSIX alike. diff --git a/tests/test_conns_export.py b/tests/test_conns_export.py index 0026e34..b5fa828 100644 --- a/tests/test_conns_export.py +++ b/tests/test_conns_export.py @@ -214,3 +214,35 @@ def test_export_csv_stats_appends_then_rotates_on_a_column_change(): if n != "stats.csv" and n.endswith(".csv")] assert len(backups) == 1, backups # the old file was kept aside ''') + + +def test_both_exports_tell_the_user_the_whole_path(): + """The name alone stopped being an answer when the files left the exe's folder. + + Both exports used to log ``os.path.basename(...)``, which was enough while the + file landed next to the executable the user had just double-clicked. It is not + enough now, and nothing else on screen names the directory. + """ + run_gui(''' + import os, tempfile + import beantester.gui.csv_export as m + folder = tempfile.mkdtemp() + m.CSV_FILE = os.path.join(folder, "stats.csv") + m.CONNECTIONS_CSV_FILE = os.path.join(folder, "conns.csv") + + lines = [] + app.log = lambda text: lines.append(str(text)) + + app.engine.stats_snapshot = lambda: {"seen": 1} + app.export_csv() + + app.engine.connections_snapshot = lambda limit=None: [] + app.conn_query = "" + app.conn_sort = {"col": "up", "reverse": True} + app.export_connections_csv() + + for name in ("stats.csv", "conns.csv"): + said = [l for l in lines if name in l] + assert said, (name, lines) + assert any(folder in l for l in said), (name, said) + ''') diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 675f60e..53c7202 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -71,6 +71,15 @@ "new": ' return bool(key == "duration" and getattr(self.app, "running", False))', "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + # The message that answers "where is my file". It was the basename while the + # file sat next to the exe, and nothing else on screen names the directory. + "label": "csv: the export log names the file but not where it went", + "file": "beantester/gui/csv_export.py", + "old": "app.log(f\"{T('log.conns_saved_to')} {path} ({len(rows)})\")", + "new": "app.log(f\"{T('log.conns_saved_to')} {os.path.basename(path)} ({len(rows)})\")", + "test": "test_both_exports_tell_the_user_the_whole_path", + }, { # The whole point of moving the user files: a package manager owns the # install directory and wipes it on upgrade. This is the old behaviour From c33af23cfaed8eea3796fedc233bc4c4a3e9f23e Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 17:42:02 +0200 Subject: [PATCH 3/7] feat(packaging): Chocolatey and WinGet sources, as templates nobody has to retype Everything under packaging/ is a template with {{PLACEHOLDER}} markers, filled by tools/build_packages.py from the one place that owns each value: the version from VERSION.txt, the checksum AND the archive name from the release's own SHA256SUMS.txt, the release date from the dated changelog section, the URLs from site/site.json, and the identity and nested exe path from appinfo. A version typed into a manifest is a second source of truth (convention 34), and a manifest with a stale one still parses. Nothing is published and nothing is wired into release.yml: submitting to a feed is a human step, because a bad manifest costs somebody else's moderation queue. packaging/README.md carries the order the steps go in, including the one that matters most - the first published manifest must point at a release that already keeps user files in %LOCALAPPDATA%, or WinGet's upgrade destroys them before the version that migrates them ever runs. Checked at the source rather than taken from notes: the newest manifest schema is 1.28.0, and ArchiveBinariesDependOnPath adds the directory HOLDING the nested file to PATH, which is what the implementation does and what the field's description does not say. Verified with the real tools against the published 0.4.0 checksum: winget validate reports success and choco pack builds the nupkg. That run also found a real bug - sha256sum's binary-mode star was riding into the installer URL. Guard: tests/test_packaging.py, ten checks, three mutations registered. One of them survived at first, because the check was satisfied by a comment after the code it named was gone. Co-Authored-By: Claude Opus 5 --- packaging/README.md | 57 ++++++ .../chocolatey/bean-network-tester.nuspec.in | 45 +++++ .../tools/chocolateybeforemodify.ps1.in | 28 +++ .../chocolatey/tools/chocolateyinstall.ps1.in | 28 +++ packaging/winget/installer.yaml.in | 28 +++ packaging/winget/locale.en-US.yaml.in | 34 ++++ packaging/winget/version.yaml.in | 8 + tests/test_mutation_registry.py | 29 +++ tests/test_packaging.py | 145 ++++++++++++++ tools/build_packages.py | 179 ++++++++++++++++++ 10 files changed, 581 insertions(+) create mode 100644 packaging/README.md create mode 100644 packaging/chocolatey/bean-network-tester.nuspec.in create mode 100644 packaging/chocolatey/tools/chocolateybeforemodify.ps1.in create mode 100644 packaging/chocolatey/tools/chocolateyinstall.ps1.in create mode 100644 packaging/winget/installer.yaml.in create mode 100644 packaging/winget/locale.en-US.yaml.in create mode 100644 packaging/winget/version.yaml.in create mode 100644 tests/test_packaging.py create mode 100644 tools/build_packages.py diff --git a/packaging/README.md b/packaging/README.md new file mode 100644 index 0000000..2bfa86c --- /dev/null +++ b/packaging/README.md @@ -0,0 +1,57 @@ +# Package sources: Chocolatey and WinGet + +These are **templates**, not packages. Every `{{PLACEHOLDER}}` is filled by +`tools/build_packages.py` from the one place that owns the value: the version from +`VERSION.txt`, the checksum and the archive's name from the release's own +`SHA256SUMS.txt`, the URLs from `site/site.json`, and the name, publisher, licence +and copyright from `beantester/appinfo.py`. Nothing here is typed twice, which is +why nothing here can go stale on its own. + + python tools/build_packages.py --sums SHA256SUMS.txt + +The rendered files land in `build/packaging/` and are not tracked. + +## The order these steps have to happen in + +🔴 **The first published manifest must point at a release that keeps user files in +`%LOCALAPPDATA%`.** Builds up to 0.4.0 wrote profiles, window state and the CSV +exports next to the executable, and a WinGet upgrade deletes the extracted directory +before installing the new one - so publishing an older version first would destroy +the files of everyone who installed it, on their very first upgrade, before the +version that knows how to migrate them ever ran. Publish with the release that +carries the move, not before it. + +1. Tag and publish the release the normal way (PROJECT_NOTES, "Wydanie"). +2. Download that release's `SHA256SUMS.txt` and render: + `python tools/build_packages.py --sums SHA256SUMS.txt`. +3. Chocolatey: `choco pack build/packaging/chocolatey/bean-network-tester.nuspec`, + then `choco install bean-network-tester -s . -y` on a machine you can break, then + `choco push` with an API key. Moderation is a validator, an automated verifier + that installs it in a VM, and a human. +4. WinGet: `winget validate --manifest build/packaging/winget`, then + `winget install --manifest build/packaging/winget` locally, then open a pull + request against `microsoft/winget-pkgs` with the three files under + `manifests/d/DonislawDev/BeanNetworkTester//`. + +**Submitting is a human step and stays one.** Neither of these is wired into +`release.yml`: a bad manifest is public and moderated, and the cost of catching it +after the fact is somebody else's review time. + +## What each package has to get right + +**Chocolatey.** It downloads the release archive rather than embedding it, so the +package carries no binaries and owes no `VERIFICATION.txt`. `chocolateybeforemodify.ps1` +releases the WinDivert driver before an upgrade or an uninstall, because the kernel +holds `WinDivert64.sys` open while it is loaded and an open file cannot be deleted. +Its package folder is read-only for plain users, which is one of the two reasons the +program stopped keeping user files in its own directory. + +**WinGet.** `ArchiveBinariesDependOnPath: true` is the line that matters. The default +for a portable inside an archive is a symlink, and this executable cannot be reached +through one - it needs the `_internal` directory beside it. The field puts the +directory holding the nested file on `PATH` instead, which is what winget's source +does with it rather than what the field's one-line description implies. + +Neither manifest can keep a file safe on its own: WinGet portables take no scripts at +all, and that is why the program itself had to stop writing into the directory the +package manager owns. diff --git a/packaging/chocolatey/bean-network-tester.nuspec.in b/packaging/chocolatey/bean-network-tester.nuspec.in new file mode 100644 index 0000000..692d16d --- /dev/null +++ b/packaging/chocolatey/bean-network-tester.nuspec.in @@ -0,0 +1,45 @@ + + + + + {{CHOCO_ID}} + {{VERSION}} + {{REPO_URL}}/tree/master/packaging/chocolatey + {{PUBLISHER}} + {{APP_NAME}} + {{PUBLISHER}} + {{PROJECT_URL}} + {{REPO_URL}}/raw/master/bean.png + {{COPYRIGHT}} + {{LICENSE_URL}} + false + {{REPO_URL}} + {{REPO_URL}}#readme + {{REPO_URL}}/issues + network testing qa latency packet-loss bandwidth windivert admin + {{TAGLINE}} + + {{RELEASE_NOTES_URL}} + + + + + diff --git a/packaging/chocolatey/tools/chocolateybeforemodify.ps1.in b/packaging/chocolatey/tools/chocolateybeforemodify.ps1.in new file mode 100644 index 0000000..3411b13 --- /dev/null +++ b/packaging/chocolatey/tools/chocolateybeforemodify.ps1.in @@ -0,0 +1,28 @@ +# Rendered by tools/build_packages.py - do not edit the generated copy. +# +# Chocolatey runs this from the EXISTING package before an upgrade or an uninstall. +# It exists for one reason: the WinDivert driver. +# +# While the driver is loaded the kernel holds WinDivert64.sys open, and a file that +# is open cannot be deleted - so the file removal that upgrading does would fail +# part-way through and leave a half-replaced package. The program already releases +# the driver when the last instance exits, so this only matters when a session is +# still running or a previous run died hard. `--cleanup-driver` is the supported way +# to ask for that cleanup, and it is a no-op when there is nothing to clean. +# +# Nothing here may throw: this script runs BEFORE the operation the user asked for, +# and refusing to upgrade because a cleanup failed would be worse than the leftover. +# Windows PowerShell 5.1 syntax only - that is what Chocolatey runs. +$ErrorActionPreference = 'Continue' +$toolsDir = Split-Path -Parent $MyInvocation.MyCommand.Definition +$exe = Join-Path $toolsDir '{{TOOL_ID}}\{{EXE_NAME}}' + +if (Test-Path $exe) { + Write-Host 'Releasing the WinDivert driver before changing the package...' + try { + & $exe --cleanup-driver | Write-Host + } catch { + Write-Host "Could not release the driver: $($_.Exception.Message)" + Write-Host 'If the upgrade complains about a file in use, close the program and retry.' + } +} diff --git a/packaging/chocolatey/tools/chocolateyinstall.ps1.in b/packaging/chocolatey/tools/chocolateyinstall.ps1.in new file mode 100644 index 0000000..020b78e --- /dev/null +++ b/packaging/chocolatey/tools/chocolateyinstall.ps1.in @@ -0,0 +1,28 @@ +# Rendered by tools/build_packages.py - do not edit the generated copy. +# +# The package DOWNLOADS the release archive instead of carrying it. Two reasons: +# the archive is the same file, checksum and all, that the release page publishes, +# and a package with binaries inside it owes the moderators a VERIFICATION.txt +# explaining where those binaries came from. +# +# Windows PowerShell 5.1 is what Chocolatey runs package scripts with, so nothing +# here uses PowerShell 7 syntax (convention 46 is about the shells WE choose). +$ErrorActionPreference = 'Stop' +$toolsDir = Split-Path -Parent $MyInvocation.MyCommand.Definition + +# Unzipping into the tools directory leaves the exe inside tools\{{TOOL_ID}}\, +# next to the _internal folder it cannot run without. Chocolatey shims the exe +# from where it lies, so the siblings stay +# reachable - unlike a symlink, which would sever them. +Install-ChocolateyZipPackage ` + -PackageName '{{CHOCO_ID}}' ` + -Url64bit '{{URL}}' ` + -Checksum64 '{{SHA256}}' ` + -ChecksumType64 'sha256' ` + -UnzipLocation $toolsDir + +Write-Host '' +Write-Host '{{APP_NAME}} needs Administrator rights to capture traffic, and asks for them itself.' +Write-Host 'Your profiles, window state and CSV exports live in %LOCALAPPDATA%\{{TOOL_ID}},' +Write-Host 'so upgrading or removing this package does not touch them.' +Write-Host 'Impairment with nothing to aim at hits every connection on this machine - aim it.' diff --git a/packaging/winget/installer.yaml.in b/packaging/winget/installer.yaml.in new file mode 100644 index 0000000..eb52dea --- /dev/null +++ b/packaging/winget/installer.yaml.in @@ -0,0 +1,28 @@ +# Rendered by tools/build_packages.py - do not edit the generated copy. +# Submitted as manifests/d/{{PUBLISHER}}/{{TOOL_ID}}/{{VERSION}}/{{WINGET_ID}}.installer.yaml +# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.{{WINGET_SCHEMA}}.schema.json + +PackageIdentifier: {{WINGET_ID}} +PackageVersion: {{VERSION}} +InstallerType: zip +NestedInstallerType: portable +NestedInstallerFiles: +- RelativeFilePath: {{NESTED_EXE}} +# 🔴 Load-bearing. The default for a portable inside an archive is a SYMLINK in the +# links folder, and this exe cannot be reached through one: it needs its sibling +# _internal directory, which the symlink leaves behind. Setting this skips the +# symlink and puts the directory holding the nested file on PATH instead - verified +# in winget's own source (PortableInstaller.cpp takes the parent of the nested +# file's path), not inferred from the field's description, which says only +# "install location". +ArchiveBinariesDependOnPath: true +# The program asks for elevation itself when it needs to capture. Saying so here +# stops winget from running the whole install elevated. +ElevationRequirement: elevatesSelf +ReleaseDate: {{RELEASE_DATE}} +Installers: +- Architecture: x64 + InstallerUrl: {{URL}} + InstallerSha256: {{SHA256_UPPER}} +ManifestType: installer +ManifestVersion: {{WINGET_SCHEMA}} diff --git a/packaging/winget/locale.en-US.yaml.in b/packaging/winget/locale.en-US.yaml.in new file mode 100644 index 0000000..3794561 --- /dev/null +++ b/packaging/winget/locale.en-US.yaml.in @@ -0,0 +1,34 @@ +# Rendered by tools/build_packages.py - do not edit the generated copy. +# yaml-language-server: $schema=https://aka.ms/winget-manifest.defaultLocale.{{WINGET_SCHEMA}}.schema.json + +PackageIdentifier: {{WINGET_ID}} +PackageVersion: {{VERSION}} +PackageLocale: en-US +Publisher: {{PUBLISHER}} +PublisherUrl: {{REPO_URL}} +PublisherSupportUrl: {{REPO_URL}}/issues +PackageName: {{APP_NAME}} +PackageUrl: {{PROJECT_URL}} +License: {{LICENSE}} +LicenseUrl: {{LICENSE_URL}} +Copyright: {{COPYRIGHT}} +ShortDescription: {{TAGLINE}} +Description: >- + {{DESCRIPTION}} + It adds latency, jitter, packet loss, corruption, duplication and bandwidth limits + to the traffic of a chosen application, and reports every outcome as an exit code + so it can run from a pipeline. It needs Administrator rights to capture traffic and + asks for them itself. Profiles, window state and CSV exports are kept in + %LOCALAPPDATA%\{{TOOL_ID}}, so upgrading the package leaves them alone. +Moniker: {{CHOCO_ID}} +Tags: +- bandwidth +- latency +- network +- packet-loss +- qa +- testing +- windivert +ReleaseNotesUrl: {{RELEASE_NOTES_URL}} +ManifestType: defaultLocale +ManifestVersion: {{WINGET_SCHEMA}} diff --git a/packaging/winget/version.yaml.in b/packaging/winget/version.yaml.in new file mode 100644 index 0000000..1759d9a --- /dev/null +++ b/packaging/winget/version.yaml.in @@ -0,0 +1,8 @@ +# Rendered by tools/build_packages.py - do not edit the generated copy. +# yaml-language-server: $schema=https://aka.ms/winget-manifest.version.{{WINGET_SCHEMA}}.schema.json + +PackageIdentifier: {{WINGET_ID}} +PackageVersion: {{VERSION}} +DefaultLocale: en-US +ManifestType: version +ManifestVersion: {{WINGET_SCHEMA}} diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 53c7202..ff179ef 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -71,6 +71,35 @@ "new": ' return bool(key == "duration" and getattr(self.app, "running", False))', "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + # Without this field WinGet reaches the exe through a symlink, which severs + # it from the _internal directory it cannot run without. The package would + # install cleanly and then fail to start. + "label": "packaging: the winget manifest drops ArchiveBinariesDependOnPath", + "file": "packaging/winget/installer.yaml.in", + "old": "ArchiveBinariesDependOnPath: true\n", + "new": "", + "test": "test_the_winget_manifest_keeps_the_exe_with_its_siblings", + }, + { + # Convention 34 in the place it is easiest to break: a manifest with a + # hand-typed version still parses, and still points at the wrong build. + "label": "packaging: a version number is typed into a manifest", + "file": "packaging/winget/version.yaml.in", + "old": "PackageVersion: {{VERSION}}", + "new": "PackageVersion: 0.4.0", + "test": "test_no_package_source_carries_a_version_number", + }, + { + # This one SURVIVED at first: the guard searched the whole file, so the + # comment explaining the call satisfied it after the call itself was gone. + # The fix was to the test, which now reads only lines that invoke the exe. + "label": "packaging: the chocolatey hook stops releasing the driver", + "file": "packaging/chocolatey/tools/chocolateybeforemodify.ps1.in", + "old": "& $exe --cleanup-driver | Write-Host", + "new": "& $exe --version | Write-Host", + "test": "test_the_chocolatey_scripts_release_the_driver_before_a_change", + }, { # The message that answers "where is my file". It was the basename while the # file sat next to the exe, and nothing else on screen names the directory. diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 0000000..1d2fbf0 --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,145 @@ +"""The Chocolatey and WinGet package sources, and the renderer that fills them. + +These files are published under our name to two feeds we do not control, so the +cost of a mistake is somebody else's moderation queue and, for the two lines that +matter, a user whose install does not work at all. + +What is guarded here is what a reviewer cannot catch for us: + +* no version number is typed into a package file (convention 34 - `VERSION.txt` is + the only place a version may live, and a manifest with a stale one still parses); +* `ArchiveBinariesDependOnPath` stays set, because without it WinGet reaches the + exe through a symlink and severs it from the `_internal` directory it needs; +* the nested path is the one the release actually builds, derived from `appinfo` + rather than typed a second time; +* the Chocolatey scripts release the WinDivert driver before an upgrade, because + the kernel holds the loaded `.sys` open and an open file cannot be deleted; +* the renderer refuses the two inputs that would look fine and be wrong: an unknown + placeholder, and the previous release's `SHA256SUMS.txt`. +""" +import os +import re +import sys + +import pytest +from fakes import ROOT, check + +sys.path.insert(0, ROOT) +from beantester import appinfo # noqa: E402 +from tools import build_packages as bp # noqa: E402 + +# A real line from a real release, binary marker and all. +SUMS_LINE = ("94359ea633e2e9fbe10e02b81070208a7209de2c4c48b003d8ce4feb30876bed" + " *BeanNetworkTester-v{version}-windows-x64.zip\n") + + +def _sums(tmp_path, version): + path = tmp_path / "SHA256SUMS.txt" + path.write_text(SUMS_LINE.format(version=version), encoding="utf-8") + return str(path) + + +def _rendered(tmp_path, monkeypatch): + """Render into a throwaway directory and return {relative name: text}.""" + out = tmp_path / "out" + monkeypatch.setattr(bp, "OUT_DIR", str(out)) + bp.build(_sums(tmp_path, appinfo.__version__)) + files = {} + for base, _, names in os.walk(out): + for name in names: + path = os.path.join(base, name) + files[os.path.relpath(path, out).replace(os.sep, "/")] = \ + open(path, encoding="utf-8").read() + return files + + +def _sources(): + return [(rel, open(path, encoding="utf-8").read()) + for path, rel in bp.templates() if rel.endswith(bp.TEMPLATE_SUFFIX)] + + +# -- what must never be typed twice -------------------------------------------- # +def test_no_package_source_carries_a_version_number(): + offenders = [rel for rel, text in _sources() + if re.search(r"\b\d+\.\d+\.\d+\b", text.replace(bp.WINGET_SCHEMA, ""))] + check("packaging sources hold no version literal", not offenders, f"({offenders})") + + +def test_every_placeholder_is_known_and_every_known_placeholder_is_used(): + table = set(bp.values(appinfo.__version__, "abc", "x-v1.zip")) + used = set() + for _, text in _sources(): + used |= {m.group(1) for m in bp.PLACEHOLDER.finditer(text)} + check("no template uses a placeholder the renderer cannot fill", + not used - table, f"({sorted(used - table)})") + check("no renderer entry has stopped being used by any template", + not table - used, f"({sorted(table - used)})") + + +# -- the two lines that decide whether an install works ------------------------- # +def test_the_winget_manifest_keeps_the_exe_with_its_siblings(tmp_path, monkeypatch): + installer = _rendered(tmp_path, monkeypatch)["winget/installer.yaml"] + check("ArchiveBinariesDependOnPath is set", + "ArchiveBinariesDependOnPath: true" in installer) + check("so the exe is not reached through a symlink", + "PortableCommandAlias" not in installer, + "(an alias implies the symlink this field exists to avoid)") + + +def test_the_nested_path_is_the_one_the_release_builds(tmp_path, monkeypatch): + installer = _rendered(tmp_path, monkeypatch)["winget/installer.yaml"] + expected = f"RelativeFilePath: {appinfo.TOOL_ID}/{appinfo.EXE_NAME}" + check("the manifest points at the exe inside the archive's one directory", + expected in installer, f"({expected})") + + +def test_the_chocolatey_scripts_release_the_driver_before_a_change(tmp_path, monkeypatch): + files = _rendered(tmp_path, monkeypatch) + before = files["chocolatey/tools/chocolateybeforemodify.ps1"] + # Read the CODE, not the file. The first version of this check looked for the + # flag anywhere in the text and passed on a script whose only mention of it was + # the comment explaining why it is there - the mutation survived and said so. + calls = [line for line in before.splitlines() + if "$exe" in line and not line.strip().startswith("#")] + check("the driver is released before an upgrade or uninstall", + any("--cleanup-driver" in line for line in calls), f"({calls})") + check("and a failure there cannot abort the operation", + "$ErrorActionPreference = 'Continue'" in before and "try {" in before) + + +def test_the_download_is_checksummed(tmp_path, monkeypatch): + install = _rendered(tmp_path, monkeypatch)["chocolatey/tools/chocolateyinstall.ps1"] + check("the archive is verified against the release's own hash", + "-Checksum64 '94359ea6" in install and "-ChecksumType64 'sha256'" in install) + + +# -- the inputs that would look fine and be wrong ------------------------------- # +def test_the_binary_marker_never_reaches_the_url(tmp_path, monkeypatch): + """`sha256sum` writes ` *`, and that star is not part of the name.""" + files = _rendered(tmp_path, monkeypatch) + for name, text in files.items(): + for line in text.splitlines(): + if "://" in line: + check(f"{name} has a clean URL", "*" not in line, f"({line.strip()})") + + +def test_nothing_unfilled_survives_rendering(tmp_path, monkeypatch): + for name, text in _rendered(tmp_path, monkeypatch).items(): + check(f"{name} has no placeholder left", "{{" not in text) + + +def test_the_previous_releases_checksum_file_is_refused(tmp_path, monkeypatch): + monkeypatch.setattr(bp, "OUT_DIR", str(tmp_path / "out")) + stale = _sums(tmp_path, "0.0.1") # a real file, for the wrong release + with pytest.raises(SystemExit) as refused: + bp.build(stale) + check("the mismatch names the asset", "0.0.1" in str(refused.value), + f"({refused.value})") + + +def test_an_unknown_placeholder_is_an_error_not_an_empty_string(): + table = bp.values(appinfo.__version__, "abc", "x-v1.zip") + with pytest.raises(SystemExit) as refused: + bp.render("id: {{NOT_A_REAL_KEY}}", table, "made-up.yaml") + check("the failure names the placeholder", "NOT_A_REAL_KEY" in str(refused.value), + f"({refused.value})") diff --git a/tools/build_packages.py b/tools/build_packages.py new file mode 100644 index 0000000..553b667 --- /dev/null +++ b/tools/build_packages.py @@ -0,0 +1,179 @@ +"""Render the Chocolatey and WinGet package sources for a published release. + +Everything under ``packaging/`` is a TEMPLATE with ``{{PLACEHOLDER}}`` markers, and +this script is the only thing that fills them. That is convention 34 applied to +packaging: a version number written into a manifest by hand is a second source of +truth for ``VERSION.txt``, and it goes stale in the release where somebody forgets +it - quietly, because a manifest with the wrong version still parses. + +The checksum and the asset name come from ONE input, the release's own +``SHA256SUMS.txt``. It carries both (`` *``), so there is nothing to +retype and nothing to keep in step. + + python tools/build_packages.py --sums SHA256SUMS.txt + +Output goes to ``build/packaging/`` (git-ignored, like the website's build). What +this script does NOT do is submit anything: pushing to the Chocolatey feed or +opening a pull request against microsoft/winget-pkgs is a human step, deliberately. +See ``packaging/README.md`` for the order those steps have to happen in. +""" +import argparse +import json +import os +import re +import sys + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, ROOT) + +from beantester import appinfo # noqa: E402 + +PACKAGING_DIR = os.path.join(ROOT, "packaging") +OUT_DIR = os.path.join(ROOT, "build", "packaging") +TEMPLATE_SUFFIX = ".in" +PLACEHOLDER = re.compile(r"\{\{([A-Z0-9_]+)\}\}") + +# The Chocolatey package id and the WinGet identifier. Lower-case-with-hyphens is +# the Chocolatey convention, Publisher.Package is the WinGet one. +CHOCO_ID = "bean-network-tester" +WINGET_ID = f"{appinfo.AUTHOR}.{appinfo.TOOL_ID}" +# Pinned rather than "latest": a schema version is a contract, and a manifest that +# silently follows a moving target fails in the reviewer's CI, not ours. Checked +# against microsoft/winget-pkgs/doc/manifest/schema on 2026-08-12 (convention 47). +WINGET_SCHEMA = "1.28.0" + + +def _read_json(*parts): + with open(os.path.join(ROOT, *parts), encoding="utf-8") as f: + return json.load(f) + + +def parse_sums(path): + """`` *`` -> (hash, file). The star is sha256sum's binary marker.""" + with open(path, encoding="utf-8") as f: + for line in f: + line = line.strip() + if not line: + continue + digest, _, name = line.partition(" ") + # Order matters: the separator is whitespace and THEN the `*` binary-mode + # marker, so stripping the star first leaves it attached to a leading + # space and it rides into the URL. Measured, not imagined - it did. + name = name.strip().lstrip("*").strip() + if name.lower().endswith(".zip"): + return digest.strip(), name + raise SystemExit(f"{path}: no .zip line found") + + +def release_date(version): + """The date this version was released, read from the changelog that ships it. + + ``CHANGELOG.md`` already has to carry a dated section for ``VERSION.txt`` - a + test enforces that - so it is the one place that knows, and WinGet's + ``ReleaseDate`` reads from it rather than from a second answer typed here. + """ + wanted = re.compile(r"^##\s*\[" + re.escape(version) + r"\]\s*-\s*(\d{4}-\d{2}-\d{2})") + with open(os.path.join(ROOT, "CHANGELOG.md"), encoding="utf-8") as f: + for line in f: + found = wanted.match(line.strip()) + if found: + return found.group(1) + raise SystemExit(f"CHANGELOG.md has no dated section for {version} - close it first") + + +def values(version, digest, asset): + """Every placeholder, each read from the one place that owns it.""" + site = _read_json("site", "site.json") + home = _read_json("site", "pages", "home", "page.json")["languages"]["en"] + tagline = _read_json("site", "i18n", "en.json")["site.tagline"] + repo = site["repo_url"].rstrip("/") + tag = f"v{version}" + # Every entry here is used by a template, and a test keeps it that way in both + # directions: an unknown placeholder is a typo, a dead entry is a manifest that + # quietly stopped carrying a field. + return { + "VERSION": version, + "URL": f"{repo}/releases/download/{tag}/{asset}", + "SHA256": digest.lower(), + "SHA256_UPPER": digest.upper(), + "CHOCO_ID": CHOCO_ID, + "WINGET_ID": WINGET_ID, + "WINGET_SCHEMA": WINGET_SCHEMA, + "APP_NAME": appinfo.APP_NAME, + "TOOL_ID": appinfo.TOOL_ID, # the data folder's name, which has no spaces + "EXE_NAME": appinfo.EXE_NAME, + "PUBLISHER": appinfo.AUTHOR, + "LICENSE": appinfo.LICENSE_NAME, + "LICENSE_URL": f"{repo}/blob/master/LICENSE", + "COPYRIGHT": appinfo.COPYRIGHT, + "PROJECT_URL": site["base_url"].rstrip("/"), + "REPO_URL": repo, + "RELEASE_NOTES_URL": f"{repo}/releases/tag/{tag}", + "RELEASE_DATE": release_date(version), + "TAGLINE": tagline, + "DESCRIPTION": home["description"], + # Where the exe sits inside the archive: one top-level directory, named + # after the tool, exactly as `release.yml` zips `dist/BeanNetworkTester`. + "NESTED_EXE": f"{appinfo.TOOL_ID}/{appinfo.EXE_NAME}", + } + + +def templates(): + for base, _, names in os.walk(PACKAGING_DIR): + for name in sorted(names): + path = os.path.join(base, name) + yield path, os.path.relpath(path, PACKAGING_DIR) + + +def render(text, table, where): + unknown = sorted({m.group(1) for m in PLACEHOLDER.finditer(text)} - set(table)) + if unknown: + raise SystemExit(f"{where}: unknown placeholder(s) {unknown}") + return PLACEHOLDER.sub(lambda m: str(table[m.group(1)]), text) + + +def build(sums_path, version=None): + version = version or appinfo.__version__ + digest, asset = parse_sums(sums_path) + # The commonest way to get this wrong is to feed the PREVIOUS release's file. + # The asset name carries the tag, so the mismatch is catchable, and silently + # publishing a manifest that points at the wrong build is not recoverable. + if f"v{version}" not in asset: + raise SystemExit(f"{asset} is not the asset for v{version} - wrong SHA256SUMS.txt?") + table = values(version, digest, asset) + + written = [] + for path, relative in templates(): + # Only templates become package files. Everything else under packaging/ is + # for the person reading the repository - README.md talks ABOUT placeholders, + # and copying it would both ship it and trip the check below. + if not relative.endswith(TEMPLATE_SUFFIX): + continue + with open(path, encoding="utf-8") as f: + text = f.read() + target = os.path.join(OUT_DIR, relative)[: -len(TEMPLATE_SUFFIX)] + text = render(text, table, relative) + left = PLACEHOLDER.search(text) + if left: + raise SystemExit(f"{relative}: {left.group(0)} survived rendering") + os.makedirs(os.path.dirname(target), exist_ok=True) + with open(target, "w", encoding="utf-8", newline="\n") as f: + f.write(text) + written.append(os.path.relpath(target, ROOT)) + return written + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--sums", required=True, + help="the release's SHA256SUMS.txt (carries hash AND asset name)") + parser.add_argument("--version", default=None, + help="override VERSION.txt (for trying a past release)") + args = parser.parse_args(argv) + for name in build(args.sums, args.version): + print(name) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From ec9b8b198e39ac090af9d28eb9d9eae9420aa305 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 18:00:56 +0200 Subject: [PATCH 4/7] test(packaging): keep the package sources tracked, and say why in a guard Answering "should packaging/ be ignored like internal_tools/?" with a test rather than with a memory, because the failure of getting it wrong does not show up on the machine that has the files. Measured with the directory absent: the renderer wrote nothing and the tests failed with a bare KeyError - a missing file instead of a reason, which is the exact failure mode the tools/ versus internal_tools/ rule exists for. Three separate things need these files tracked: the packaging tests and the mutation registry read them and CI runs on a fresh clone, packageSourceUrl has to point at a source a moderator can open (CPMR0040, a Guideline), and a package source is there so somebody other than us can see what the package does to their machine. - build_packages refuses an empty template set instead of writing nothing - the render tests no longer depend on the changelog being closed for the current version: the date reader has its own test, so a version bumped mid-release reddens one test that names itself rather than five that do not Co-Authored-By: Claude Opus 5 --- tests/test_packaging.py | 41 +++++++++++++++++++++++++++++++++++++++++ tools/build_packages.py | 7 +++++++ 2 files changed, 48 insertions(+) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 1d2fbf0..1e7b4ae 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -43,6 +43,10 @@ def _rendered(tmp_path, monkeypatch): """Render into a throwaway directory and return {relative name: text}.""" out = tmp_path / "out" monkeypatch.setattr(bp, "OUT_DIR", str(out)) + # A fixed date, so these tests answer "does it render" and not "is the changelog + # closed for this version". The changelog reader has its own test below, which is + # the one that should redden when a version is bumped before its section is dated. + monkeypatch.setattr(bp, "release_date", lambda version: "2026-01-01") bp.build(_sums(tmp_path, appinfo.__version__)) files = {} for base, _, names in os.walk(out): @@ -143,3 +147,40 @@ def test_an_unknown_placeholder_is_an_error_not_an_empty_string(): bp.render("id: {{NOT_A_REAL_KEY}}", table, "made-up.yaml") check("the failure names the placeholder", "NOT_A_REAL_KEY" in str(refused.value), f"({refused.value})") + + +def test_the_release_date_comes_from_the_changelog(): + """One reader, not a second answer typed into a manifest. + + This is also the test that reddens if a version is bumped before its changelog + section is dated - deliberately alone, so that failure names itself instead of + taking the rendering tests down with it. + """ + date = bp.release_date(appinfo.__version__) + check("the date looks like a date", re.fullmatch(r"\d{4}-\d{2}-\d{2}", date), f"({date})") + + +def test_the_package_sources_are_tracked_by_git(): + """These files are not internal tooling, and three separate things need them. + + Chocolatey's moderation asks for `packageSourceUrl` to point at where the + package source lives (rule CPMR0040, a Guideline), so a private path there + would be a dead link - worse than the field being absent. The tests above read + these files, and CI runs them on a fresh clone. And the whole point of a + package source is that somebody other than us can see what the package does to + their machine. + + So `packaging/` belongs where `tools/` is, not where `internal_tools/` is: the + failure of getting this wrong does not show up here, where the files exist. It + shows up on somebody else's clone, as a missing file rather than a reason. + """ + import subprocess + sources = [rel for _, rel in bp.templates()] + check("there are package sources to check", len(sources) >= 6, f"({sources})") + for relative in sorted(sources): + path = f"packaging/{relative}".replace(os.sep, "/") + tracked = subprocess.run(["git", "ls-files", "--error-unmatch", path], + cwd=ROOT, capture_output=True, text=True) + check(f"{path} is tracked by git", tracked.returncode == 0, + "(ignored or untracked - a fresh clone and the Chocolatey moderators " + "would both find nothing)") diff --git a/tools/build_packages.py b/tools/build_packages.py index 553b667..4aa50f8 100644 --- a/tools/build_packages.py +++ b/tools/build_packages.py @@ -142,6 +142,13 @@ def build(sums_path, version=None): raise SystemExit(f"{asset} is not the asset for v{version} - wrong SHA256SUMS.txt?") table = values(version, digest, asset) + found = [rel for _, rel in templates() if rel.endswith(TEMPLATE_SUFFIX)] + if not found: + # Saying this out loud beats writing nothing and letting the caller wonder. + # The way to get here is a checkout without packaging/ - which is why a test + # keeps those files tracked. + raise SystemExit(f"{PACKAGING_DIR}: no {TEMPLATE_SUFFIX} templates found") + written = [] for path, relative in templates(): # Only templates become package files. Everything else under packaging/ is From 92d11d29799dfa5d0518b7c041645c9f567b6ab6 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 19:25:07 +0200 Subject: [PATCH 5/7] feat(gui,cli): say which folder the user's files are in The folder belongs to the Windows account the program runs as. On an account without administrator rights, agreeing to the elevation prompt runs the program as the administrator account whose password was entered, so it uses that account's folder - and nothing said so, which made saved profiles look lost. Measured rather than reasoned about: the same code resolves the folder from the account it runs as, not from the account that launched it, so the two really do differ. Passing the launcher's folder into the elevated copy was rejected: a standard user's AppData is fully controlled by that user, and pointing an elevated process at a directory a plain user controls is the documented precondition for redirecting a privileged write through a directory junction, which needs no privilege to create. Today that exposure does not exist, because the elevated copy writes into its own account's folder. - About shows it in the selectable box, next to the licence path - --doctor prints a "user files:" line and carries data_dir in its JSON report - both READMEs explain that the folder follows the account, and that an administrator can set BEAN_DATA_DIR system-wide for one shared folder - new key about.data_dir in both language files The path sits on its own line for a measured reason: that box does not wrap and has no horizontal scrollbar, and on real Tk the label and a realistic frozen path came to 644 px in a 583 px box, hiding the end of the path. Split, the widest line is 413 px. Guards: test_the_about_window_says_where_the_users_files_are and test_doctor_says_where_the_users_own_files_are, both proved by mutation and registered. The first was VACUOUS at first - it asserted the path as a substring, which another line satisfied - and the mutation is what said so. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 ++++ README.md | 8 ++++++ README.pl.md | 8 ++++++ beantester/cli.py | 11 +++++-- beantester/gui/panels/about.py | 9 +++++- lang/en.json | 1 + lang/pl.json | 1 + tests/test_cli_runtime.py | 21 ++++++++++++++ tests/test_mutation_registry.py | 27 +++++++++++++++++ tests/test_windows.py | 51 +++++++++++++++++++++++++++++++++ 10 files changed, 139 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5063619..82096dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,11 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol to an older build still finds them. The reason is updates: a package manager owns the program folder and replaces it, which would take your profiles with it. Both CSV exports now log the whole path. Set `BEAN_DATA_DIR` to keep everything somewhere else, for example on a stick. +- **"About" and `--doctor` now tell you which folder your files are in.** The folder belongs to + the Windows account the program runs as, so starting it elevated on an account that is not an + administrator uses that administrator's folder instead of yours. Nothing used to say so, which + made saved profiles look lost. Both READMEs explain it, and an administrator can set + `BEAN_DATA_DIR` system-wide to give every account one shared folder. ### Docs - **Four more guides on the website: no internet, timed scenarios, game lag and chaos testing.** diff --git a/README.md b/README.md index 05c904b..de40e2e 100644 --- a/README.md +++ b/README.md @@ -837,6 +837,14 @@ by hand or through a package manager, which replaces that folder - cannot take y it. Set `BEAN_DATA_DIR` to a folder of your choosing to keep everything somewhere else, for example on the same stick as a portable copy. +The folder belongs to the Windows account the program is running as. On an account without +administrator rights, agreeing to the elevation prompt runs the program as the administrator +account whose password was entered, and it then uses THAT account's folder - so the profiles you +saved without elevation are not the ones you see with it. "About" and `--doctor` both print the +folder in use, so you can always tell which one you are looking at. To give every account on the +machine one shared folder, an administrator can set `BEAN_DATA_DIR` as a system-wide environment +variable. + | | **Statistics** ("Export CSV", Statistics -> Live) | **Connections** ("Export connections CSV") | |---|---|---| | file | `bean_network_tester_stats.csv` | `bean_network_tester_connections.csv` | diff --git a/README.pl.md b/README.pl.md index a85e22e..4b74746 100644 --- a/README.pl.md +++ b/README.pl.md @@ -695,6 +695,14 @@ Leżą poza folderem programu po to, żeby aktualizacja - ręczna albo przez men ten folder podmienia - nie zabrała Twoich plików razem z nim. Ustaw `BEAN_DATA_DIR` na wybrany folder, żeby trzymać wszystko gdzie indziej, na przykład na tym samym pendrivie co kopia programu. +Folder należy do konta Windows, na którym program działa. Na koncie bez uprawnień administratora +zgoda na pytanie o podniesienie uprawnień uruchamia program jako to konto administratora, którego +hasło zostało podane, więc używa TAMTEGO folderu - profile zapisane bez podniesienia uprawnień to +nie są te, które widzisz z nim. Okno „O programie" i `--doctor` pokazują folder, z którego program +właśnie korzysta, więc zawsze wiadomo, na który patrzysz. Żeby wszystkie konta na maszynie miały +jeden wspólny folder, administrator może ustawić `BEAN_DATA_DIR` jako systemową zmienną +środowiskową. + | | **Statystyki** („Eksportuj CSV", Statystyki → Na żywo) | **Połączenia** („Eksportuj połączenia CSV") | |---|---|---| | plik | `bean_network_tester_stats.csv` | `bean_network_tester_connections.csv` | diff --git a/beantester/cli.py b/beantester/cli.py index e2d6297..dc7939d 100644 --- a/beantester/cli.py +++ b/beantester/cli.py @@ -25,7 +25,7 @@ from .fields import BOOL, FIELD_DEFS from .filters import CLI_FILTERS from .i18n import T -from .paths import is_frozen +from .paths import is_frozen, user_data_dir from .presets import (PRESETS, closest_preset, preset_to_settings, resolve_preset) from .repro import save_repro_report, settings_to_cli_string @@ -438,12 +438,19 @@ def _run_license(log): def _run_doctor(log): ok, checks = driver.doctor() + # Where the user's own files are is an environment fact like the rest of this + # report, and it is the only one a person cannot look up: the folder follows + # the ACCOUNT the program runs as, so it moves when the same person starts it + # elevated onto another account. Not a check - it has no pass or fail - so it + # is a line of its own rather than a made-up state in the status column. + where = user_data_dir() if log.fmt == clilog.JSON: - log.data(dict(event="doctor", ok=ok, + log.data(dict(event="doctor", ok=ok, data_dir=where, checks=[dict(check=c, state=st, detail=d) for c, st, d in checks]), "") else: for check, state, detail in checks: log.data(dict(), f"{state.upper():<4} {check:<18} {detail}") + log.data(dict(), f"user files: {where}") return exitcodes.OK if ok else exitcodes.RUNTIME diff --git a/beantester/gui/panels/about.py b/beantester/gui/panels/about.py index 7c07551..5c90cc1 100644 --- a/beantester/gui/panels/about.py +++ b/beantester/gui/panels/about.py @@ -25,6 +25,7 @@ from ...appinfo import (APP_NAME, AUTHOR, COPYRIGHT, LICENSE_NAME, SUPPORT_URL, __version__) from ...i18n import T +from ...paths import user_data_dir from ..labels import wrapping_label from ..scaling import scaled from ..theme import FIELD, FG, MONO_FONT, MUT @@ -109,7 +110,13 @@ def build(self, body): for name, version, licence, url in legal.component_rows(): text.insert("end", "%-26s %-10s %s\n%-26s %-10s %s\n" % (name, version, licence, "", "", url)) - text.insert("end", "\n" + T("about.licenses_dir", path=legal.licenses_dir()) + "\n") + # Both of these are paths on the reader's own disk, so they belong in the + # selectable box rather than in a label: the answer to "where are my + # profiles" is only useful if it can be copied into an address bar. It is + # also the only place in the program that says where they are - the folder + # follows the ACCOUNT, so it moves when the program is run as another user. + text.insert("end", "\n" + T("about.data_dir", path=user_data_dir()) + "\n") + text.insert("end", T("about.licenses_dir", path=legal.licenses_dir()) + "\n") text.config(state="disabled") def _donate(self): diff --git a/lang/en.json b/lang/en.json index 36c6e82..94b2e6c 100644 --- a/lang/en.json +++ b/lang/en.json @@ -4,6 +4,7 @@ "name": "English" }, "about.author": "Author: {author}", + "about.data_dir": "Your profiles, window state and CSV exports:\n{path}", "about.license": "Licence: {license}", "about.license_terms": "Free and open source under the GNU GPL v3. Use, change and share it under the same licence.", "about.licenses_dir": "Full licence texts: {path}", diff --git a/lang/pl.json b/lang/pl.json index 338fc0c..f6504aa 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -4,6 +4,7 @@ "name": "Polski" }, "about.author": "Autor: {author}", + "about.data_dir": "Twoje profile, stan okna i eksporty CSV:\n{path}", "about.license": "Licencja: {license}", "about.license_terms": "Wolne i otwarte oprogramowanie na licencji GNU GPL v3. Wolno używać, zmieniać i udostępniać dalej na tej samej licencji.", "about.licenses_dir": "Pełne teksty licencji: {path}", diff --git a/tests/test_cli_runtime.py b/tests/test_cli_runtime.py index 17fd372..9b2e1e2 100644 --- a/tests/test_cli_runtime.py +++ b/tests/test_cli_runtime.py @@ -875,6 +875,27 @@ def test_print_config_dumps_the_effective_settings(): check("--print-config: duration is part of the model", "duration" in settings) +def test_doctor_says_where_the_users_own_files_are(): + """The one environment fact a person cannot look up for themselves. + + The folder follows the ACCOUNT the program runs as, so the same person gets a + different one when they start it elevated onto another account. Nothing else in + the program says where it is, which is what made that silent. + + No admin rights needed: this line is printed whatever the driver checks decide, + which is also why it is a line rather than a check with a state. + """ + from beantester.paths import user_data_dir + _, out, _ = cli(["--doctor"]) + check("--doctor: names the user's data directory", user_data_dir() in out, + f"({out[-300:]})") + + _, js, _ = cli(["--doctor", "--format", "json"]) + payload = json.loads(js.strip().splitlines()[0]) + check("--doctor --format json: carries it as a field", + payload.get("data_dir") == user_data_dir(), f"({payload.get('data_dir')})") + + def test_doctor_reports_the_environment(): code, out, _ = cli(["--doctor"]) # The two lines it prints are true on any machine, elevated or not. diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index ff179ef..bbce087 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -71,6 +71,33 @@ "new": ' return bool(key == "duration" and getattr(self.app, "running", False))', "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + # The only place the interface answers "where are my profiles". This guard + # was VACUOUS at first: it asserted the path alone, and from sources the + # data directory is the project root, which is a prefix of the licence-texts + # path shown two lines below - so another line satisfied it. It now asserts + # the whole rendered sentence. + "label": "about: the window stops naming the user's data directory", + "file": "beantester/gui/panels/about.py", + "old": ' text.insert("end", "\\n" + T("about.data_dir", ' + 'path=user_data_dir()) + "\\n")\n', + "new": "", + "test": "test_the_about_window_says_where_the_users_files_are", + }, + { + "label": "doctor: stops printing where the user's files are", + "file": "beantester/cli.py", + "old": ' log.data(dict(), f"user files: {where}")\n', + "new": "", + "test": "test_doctor_says_where_the_users_own_files_are", + }, + { + "label": "doctor: the JSON report loses the data_dir field", + "file": "beantester/cli.py", + "old": 'log.data(dict(event="doctor", ok=ok, data_dir=where,', + "new": 'log.data(dict(event="doctor", ok=ok,', + "test": "test_doctor_says_where_the_users_own_files_are", + }, { # Without this field WinGet reaches the exe through a symlink, which severs # it from the _internal directory it cannot run without. The package would diff --git a/tests/test_windows.py b/tests/test_windows.py index 8c43f1d..2f21f02 100644 --- a/tests/test_windows.py +++ b/tests/test_windows.py @@ -463,3 +463,54 @@ def texts(widget, out): assert needle.split("{")[0][:30] in shown or needle in shown, \ "the About window does not show %s: %r" % (what, shown[:400]) """) + + +def test_the_about_window_says_where_the_users_files_are(): + """The only place in the interface that answers "where are my profiles". + + They stopped living next to the executable, because a package manager owns + that directory and replaces it on upgrade. The window is also the only place + that can make the other half visible: the folder follows the ACCOUNT, so the + same person elevating onto a different account gets a different one. + + Asserted through the rendered widget contents, not the language file, so a key + that exists and is never shown does not pass. It goes into the selectable Text + box on purpose - a path is only an answer if it can be copied. + """ + run_gui(""" + import beantester as bnt + from beantester.paths import user_data_dir + + panel = app.open_window("about") + assert panel is not None, "the About window did not open" + + def contents(widget, out): + out.append(str(widget.kw.get("text", ""))) + lines = getattr(widget, "lines", None) + if isinstance(lines, list): + out.extend(str(l) for l in lines) + for child in getattr(widget, "children", []): + contents(child, out) + return out + + items = [str(i) for i in contents(panel.win, [])] + shown = " | ".join(items) + + # The path gets a line of its OWN, and that is load-bearing rather than + # tidy: the box is `wrap="none"` with no horizontal scrollbar, so a line + # wider than the box is cut and unreachable. Measured on real Tk with a + # realistic frozen path - label and path together came to 644 px in a + # 583 px box, hiding the end of the path, which is the part that matters. + # Split, the widest line is 413 px. + label, newline, path = bnt.T("about.data_dir", path=user_data_dir()).partition(chr(10)) + assert newline and path, "the path must sit on its own line or it gets cut off" + assert label.strip() in shown, \ + "the About window does not say where the user's files are: %r" % shown[:400] + + # Compared as a WHOLE entry, not as a substring: from sources the data + # directory is the project root, which is also a prefix of the licence-texts + # path shown right below - so a substring check passed with this line + # deleted, satisfied by somebody else's line. The mutation said so. + assert any(i.strip() == path for i in items), \ + "the path is not shown on its own: %r" % shown[:400] + """) From 23822180ab01ea6e6cc71d83bbf519a11a632943 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 19:39:32 +0200 Subject: [PATCH 6/7] fix(packaging): rendering onto another drive is not a crash os.path.relpath raises ValueError on Windows when the two paths sit on different drives, and build_packages used it only to print what it had written. Found by CI rather than here: the Windows runner keeps the repository on one drive and the temporary directory on another, so six packaging tests failed there while passing on this machine, where both are on C:, and on Linux, where drives do not exist. Rendering into a directory on another volume is a legitimate thing to ask a build tool for. Guard: test_rendering_onto_another_drive_is_not_a_crash. The runner's condition was also reproduced locally, by making only that one call raise. Co-Authored-By: Claude Opus 5 --- tests/test_packaging.py | 17 +++++++++++++++++ tools/build_packages.py | 17 ++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 1e7b4ae..1a50738 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -184,3 +184,20 @@ def test_the_package_sources_are_tracked_by_git(): check(f"{path} is tracked by git", tracked.returncode == 0, "(ignored or untracked - a fresh clone and the Chocolatey moderators " "would both find nothing)") + + +def test_rendering_onto_another_drive_is_not_a_crash(monkeypatch): + """Windows raises when two paths are on different drives, and CI is that case. + + The repository sits on one drive on the Windows runner and the temporary + directory on another, so `os.path.relpath` - used only to print what was + written - raised `ValueError` and took six tests with it. It passed on this + machine, where both are on C:, and on Linux, where drives do not exist. + """ + def different_drive(path, start): + raise ValueError("path is on mount 'C:', start on mount 'D:'") + + monkeypatch.setattr(os.path, "relpath", different_drive) + shown = bp.display_path(os.path.join("X:", "out", "installer.yaml")) + check("it falls back to the absolute path instead of raising", + shown.endswith("installer.yaml"), f"({shown})") diff --git a/tools/build_packages.py b/tools/build_packages.py index 4aa50f8..8d3ac01 100644 --- a/tools/build_packages.py +++ b/tools/build_packages.py @@ -118,6 +118,21 @@ def values(version, digest, asset): } +def display_path(path): + """``path`` relative to the repository, or absolute when that is impossible. + + ``os.path.relpath`` RAISES on Windows when the two paths sit on different + drives, and this is only ever used to print what was written. Rendering into + a directory on another volume is a legitimate thing to ask for, and it is + what the Windows CI runner does by default - the repository is on one drive + and the temporary directory on another, which is how this was found. + """ + try: + return os.path.relpath(path, ROOT) + except ValueError: + return path + + def templates(): for base, _, names in os.walk(PACKAGING_DIR): for name in sorted(names): @@ -166,7 +181,7 @@ def build(sums_path, version=None): os.makedirs(os.path.dirname(target), exist_ok=True) with open(target, "w", encoding="utf-8", newline="\n") as f: f.write(text) - written.append(os.path.relpath(target, ROOT)) + written.append(display_path(target)) return written From 363722232cf4618fcb0a32916efc04b848a827c4 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Wed, 12 Aug 2026 19:48:45 +0200 Subject: [PATCH 7/7] test(packaging): register the cross-drive mutation instead of claiming the guard The changelog said "guard: test_rendering_onto_another_drive_is_not_a_crash", and an unproven claim of that shape is exactly what the registry exists to stop: it should either be re-runnable or be listed as unproven out loud. Proved: removing the fallback in display_path reddens that test and nothing else. Co-Authored-By: Claude Opus 5 --- tests/test_mutation_registry.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index bbce087..4f632fb 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -71,6 +71,17 @@ "new": ' return bool(key == "duration" and getattr(self.app, "running", False))', "test": "test_start_only_fields_are_locked_while_a_session_runs", }, + { + # The one defect in this work that CI found and this machine could not: + # relpath raises across drives, and the Windows runner keeps the repo and + # the temp directory on different ones. + "label": "packaging: the renderer raises when its output is on another drive", + "file": "tools/build_packages.py", + "old": " try:\n return os.path.relpath(path, ROOT)\n" + " except ValueError:\n return path\n", + "new": " return os.path.relpath(path, ROOT)\n", + "test": "test_rendering_onto_another_drive_is_not_a_crash", + }, { # The only place the interface answers "where are my profiles". This guard # was VACUOUS at first: it asserted the path alone, and from sources the