diff --git a/CHANGELOG.md b/CHANGELOG.md index 80b9f64..a845af8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,28 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ## [Unreleased] +### Added + +- **A "Reordered" figure on the Statistics page, and a `packets_reordered` column in the + stats CSV.** Jitter and the latency spike change the order packets go out in, but nothing + ever said whether that actually happened, and it depends on how busy the traffic is rather + than on the setting alone. Zero with jitter set now means the packets were too far apart to + overtake each other, instead of looking the same as an application that coped. An existing + stats CSV is rotated to a dated backup the first time the new column is written. + +### Changed + +- **The "Latency (ping)" card is now "Latency (ping) and packet order", and the spike + tooltip says what the spike is good for.** Delaying a slice of packets is how you test a + protocol that has to survive out-of-order delivery, and the field search matches names + rather than tooltips - so searching the Control page for "order" (or "kolejnosc") found + nothing at all, in either language. It now finds the card and all four fields in it. +- **A speed unit you can pick: `KB/s`, `Mbit/s` or `MB/s` (Settings window).** The Statistics + page, the chart, the session peak and average, and a grey readout beside Download and Upload all + follow it. It changes what you READ, never what you type - the limits stay in KB/s, which is what + a saved config file, the schedule, the scenarios, `--down`/`--up` and the NDJSON output carry. + `K` here is 1024 and a megabit is a decimal million, so 1024 KB/s reads as 8.39 Mbit/s, not 8. + ## [0.6.0] - 2026-09-03 ### Added diff --git a/README.md b/README.md index c79966b..765bf73 100644 --- a/README.md +++ b/README.md @@ -267,6 +267,19 @@ KB/s. 0 = no limit. Ping is small packets, so a speed limit barely changes it - use a file download. A positive value always limits something: an extremely small limit (below 1 B/s) is floored to 1 B/s, it does not silently turn into "no limit". +**Speed unit (Settings window)** - whether the Statistics page, the chart and the readout next to +these two fields show throughput in `KB/s`, `Mbit/s` or `MB/s`. It changes what you READ, never +what you type: the limits themselves stay in KB/s, because that is the number a saved +configuration file, the throughput schedule, the shipped scenarios, `--down`/`--up` and the NDJSON +output all carry. Pick a unit and a grey `(= 8.39 Mbit/s)` appears beside the field, so you can +type the number the tool wants while reading the number your link is sold in. + +> **`K` here is 1024, and a megabit is a decimal million** - the two conventions do not cancel out, +> so 1024 KB/s is **8.39 Mbit/s**, not 8. That is the honest conversion, not a rounding error: a +> byte is 8 bits, `1024 x 1024 x 8 = 8 388 608` bits per second, and megabit means 10^6 bits +> everywhere a link is sold. Divide by 8.39, not by 8, when you want a limit to match an +> advertised speed. + **Buffer** - the capacity of the link buffer for a speed limit, in milliseconds (0 = unlimited buffer). It sets how much queueing delay may build up on a rate-limited link before it starts dropping the excess (bufferbloat). Without this buffer the token bucket could "run away" tens of @@ -299,6 +312,19 @@ one. With the given probability it appends extra delay (ms) to a **single packet momentary "lag" actually arrives. The chance is per packet and applies **in each direction**, so a round trip hits it about twice as often as the number suggests. +**Testing out-of-order delivery** - this is what the spike pair is for, and it is the reason the +section is called "Latency (ping) and packet order". A spiked packet arrives after packets that +were sent later than it, so *Spike chance* and *Spike size* reorder traffic **without** smearing +every other packet's delay the way jitter does - which matters when the thing under test is a UDP +protocol that has to survive reordering on its own: a game, QUIC, telemetry, voice. Set a spike +size larger than the gap between your packets, or nothing will overtake anything. + +Whether that actually happened is a separate question from whether it was configured, so the tool +counts it: **Reordered** on the Statistics page (and `packets_reordered` in the stats CSV). 0 with a +spike set means your traffic was too sparse for any packet to overtake another - not that the +setting was ignored. The count is per direction, so two busy connections can overtake each other +without either end seeing anything out of order. + **Impairments** - *Loss*: percentage of packets vanishing without a trace (5% is already a clearly failing network). *Corruption*: percentage of packets with a flipped data bit - it affects **only payload-bearing packets**. Packets with no data (e.g. pure ACK, SYN) have nothing to flip, @@ -947,6 +973,7 @@ what `packets_seen` counted in the first place - so every row records it in `cap | `dropped_overflow` | dropped because the tool's own queue was full (see the note on it below) | | `corrupted` | packets whose payload was flipped | | `duplicated` | extra copies queued | +| `packets_reordered` | packets that left the tool in a different order than they went in. 0 with jitter or a spike configured means the packets were too far apart for any of them to overtake another. Counted per direction, so two busy connections can overtake each other without either end seeing anything out of order | | `dropped_syn` | TCP SYNs dropped ("connections that never open") | | `dropped_mtu` | dropped for exceeding the max size (MTU black hole) | | `dropped_nat` | dropped because the NAT mapping had expired | @@ -1262,6 +1289,7 @@ the root, so all existing commands (README, reproduction reports, PyInstaller) w bean_network_tester.py launcher + compatibility facade (re-exports the public API) beantester/ the implementation package core.py pure per-packet decision core (BeanCore) + damage.py how much a session damaged: drop reasons, loss/corruption shares engine.py capture/inject threads, statistics (BeanEngine) matchers.py filter expressions (list/range/!/>/ now: self._cv.wait(timeout=min(release - now, 0.5)) @@ -2057,6 +2050,8 @@ def _inject_loop(self): # heading the session panel used for delivered: measured # bytes_in = 5 122 600 B in a row that received 409 600 B. self._log_delivered(key, size, is_out) + # AFTER the send, deliberately - see _note_order. + self._note_order(arrived, bool(is_out)) except Exception as e: # The packet is already off the heap: not delivered, and until this # counter existed, not recorded either - it simply left the diff --git a/beantester/gui/chart.py b/beantester/gui/chart.py index 9307165..a589eb3 100644 --- a/beantester/gui/chart.py +++ b/beantester/gui/chart.py @@ -1,6 +1,14 @@ -"""Throughput chart: grid, Y axis in KB/s, down/up series and live readouts.""" +"""Throughput chart: grid, Y axis, down/up series and live readouts. + +The series are KB/s, which is what the engine counts and what the caller +holds. The Y axis is LABELLED in whichever unit the reader picked (see +``gui/rates.py``), so the picture is the same and only the numbers beside it +change - the docstring used to say "Y axis in KB/s" and that stopped being +true the moment the preference existed. +""" from ..i18n import T from ..utils import nice_ceiling +from .rates import BASE_LABEL, DEFAULT_UNIT, UNIT_LABEL, in_unit from .scaling import chart_geometry, scaled from .theme import DOWN_C, FONT, GRID_C, MUT, UP_C @@ -18,8 +26,16 @@ def _axis_label(value, peak): return f"{value:.2f}" -def draw_throughput_chart(canvas, down_hist, up_hist, sample_interval_s=0.7): - """Redraw the throughput chart on the given canvas.""" +def draw_throughput_chart(canvas, down_hist, up_hist, sample_interval_s=0.7, + unit=DEFAULT_UNIT): + """Redraw the throughput chart on the given canvas. + + ``unit`` changes the AXIS LABELS and the caption only. The history and the + plot scale stay in KB/s, which is what the caller holds and what the + engine counts - converting the data as well would mean the "nice" ceiling + were computed on one scale and drawn on another, and the shape of the line + is the same either way. + """ c = canvas try: width = c.winfo_width() @@ -46,10 +62,11 @@ def draw_throughput_chart(canvas, down_hist, up_hist, sample_interval_s=0.7): y = y0 + ph - ph * frac c.create_line(x0, y, x0 + pw, y, fill=GRID_C) c.create_text(g["ml"] - scaled(8), y, anchor="e", fill=MUT, font=(FONT, 8), - text=_axis_label(peak * frac, peak)) + text=_axis_label(in_unit(peak * frac, unit), + in_unit(peak, unit))) # the unit caption sits ABOVE the plot, not on top of the topmost value c.create_text(scaled(6), y0 - scaled(12), anchor="w", fill=MUT, font=(FONT, 8), - text="KB/s") + text=UNIT_LABEL.get(unit, BASE_LABEL)) # X axis labels (time) - inside the bottom margin, not clipped by the edge n = len(down_hist) diff --git a/beantester/gui/csv_export.py b/beantester/gui/csv_export.py index e03e593..3c355a1 100644 --- a/beantester/gui/csv_export.py +++ b/beantester/gui/csv_export.py @@ -22,6 +22,7 @@ # by spreadsheets, so it gets column names that mean something. CSV_COLUMNS = {"seen": "packets_seen", "scoped_seen": "packets_in_scope", "drop_loss": "dropped_loss", "loss_bursts": "loss_runs", + "reordered": "packets_reordered", "drop_overflow": "dropped_overflow", "drop_syn": "dropped_syn", "drop_mtu": "dropped_mtu", "drop_nat": "dropped_nat", "drop_rst": "dropped_rst", "rst_reset": "connections_reset", diff --git a/beantester/gui/form.py b/beantester/gui/form.py index f2a38ff..efbe1ab 100644 --- a/beantester/gui/form.py +++ b/beantester/gui/form.py @@ -30,6 +30,7 @@ from . import dialogs from .accordion import CollapsibleSection from .labels import wrapping_label +from .rates import DEFAULT_UNIT, RATE_FIELD_KEYS, rate_with_unit from .scaling import scaled from .theme import popdown_height, unhighlight_combobox from .tooltip import add_tooltip @@ -107,6 +108,7 @@ def __init__(self, parent, app, extras=None, scroller=None, sections=None, self.errors = {} # section id -> error label (packed only when set) self.notes = {} # section id -> override note label self.helps = {} # settings key -> its "?" cheat-sheet button + self.rate_hints = {} # rate field key -> its converted-value label self._invalid = set() # section ids whose fields currently fail validation self.columns = 1 self.column_frames = [] @@ -114,6 +116,7 @@ def __init__(self, parent, app, extras=None, scroller=None, sections=None, self.host = ttk.Frame(parent) self.host.pack(fill="both", expand=True) self._build() + self.sync_rate_hints() self.host.bind("", self._on_host_configure) # -- construction -------------------------------------------------------- # @@ -121,6 +124,7 @@ def _build(self): app = self.app self.sections, self.entries, self.labels = {}, {}, {} self.errors, self.notes, self.helps = {}, {}, {} + self.rate_hints = {} # Real column FRAMES, not grid columns: in a grid the row height is # shared across columns, so one tall section on the left blew a hole @@ -293,9 +297,49 @@ def _place_one(self, row, field, sec): if field.hint: ttk.Label(cell, text=T(field.hint), style="Hint.TLabel").pack( side="left", padx=(scaled(8), 0)) + if field.key in RATE_FIELD_KEYS: + # The same number in the unit the reader picked. The ENTRY stays in + # KB/s because that is what a config file, the schedule string and the + # command line all carry (see gui/rates.py), so this is the one place + # that says what the typed number means in Mbit/s or MB/s. Empty while + # the preference IS KB/s: repeating the value beside itself is noise. + hint = ttk.Label(cell, text="", style="Hint.TLabel") + hint.pack(side="left", padx=(scaled(8), 0)) + self.rate_hints[field.key] = hint entry.bind("", lambda e, s=sec.id: self._on_edit(s), add="+") entry.bind("", lambda e, s=sec.id: self._on_edit(s), add="+") + if field.key in RATE_FIELD_KEYS: + entry.bind("", lambda e: self.sync_rate_hints(), add="+") + entry.bind("", lambda e: self.sync_rate_hints(), add="+") + + def sync_rate_hints(self): + """Rewrite "(= 8.39 Mbit/s)" beside every rate field. Cheap, and idempotent. + + Called on every keystroke in one of those fields and by ``App.set_pref`` + when the unit changes, rather than on a timer: the two things that can + move it are a typed character and a dropdown, and both are events. With + the base unit selected the labels go EMPTY - a value repeated beside + itself is noise, not information. + """ + unit = self.app.pref("rate_unit") + for key, label in self.rate_hints.items(): + if not label.winfo_exists(): + continue + var = self.app.vars.get(key) + if unit == DEFAULT_UNIT or var is None: + label.config(text="") + continue + # A half-typed value is not an error here: this readout is a comment on + # what is in the box, so while the box says "12x" it simply says nothing + # rather than colouring the field or logging. + try: + kbps = float(str(var.get()).strip() or 0) + except (TypeError, ValueError): + label.config(text="") + continue + label.config(text=T("fields.rate_converted").format( + value=rate_with_unit(kbps, unit))) # -- events -------------------------------------------------------------- # def _show_match_help(self): diff --git a/beantester/gui/pages/control.py b/beantester/gui/pages/control.py index c50c9d3..6f2b9c6 100644 --- a/beantester/gui/pages/control.py +++ b/beantester/gui/pages/control.py @@ -178,6 +178,15 @@ def on_pref_changed(self, key): """A preference was written in the Settings window (see gui/pages).""" if key == "show_control_search": self._sync_search_visibility() + elif key == "rate_unit": + # The converted readout beside Download and Upload. PUSHED rather than + # left to the next rebuild: the dropdown that moves it lives in another + # window, and a label still naming the old unit reads as the preference + # having been ignored. It belongs on the PAGE rather than in + # ``App.set_pref`` for the reason ``pref_changed`` gives about itself - + # gui/app.py sits ON the size ratchet with no headroom, so a reaction + # put there has to come straight back out. + self.form.sync_rate_hints() def _sync_search_visibility(self): """Bring the bar in or out, once per actual change.""" diff --git a/beantester/gui/pages/stats.py b/beantester/gui/pages/stats.py index 9104fc8..2fbaa9e 100644 --- a/beantester/gui/pages/stats.py +++ b/beantester/gui/pages/stats.py @@ -13,12 +13,12 @@ import tkinter as tk from tkinter import ttk -from ...engine import impairment_loss_pct +from ...damage import impairment_loss_pct from ...i18n import T, event_kind_label from ...views import sort_events from ..chart import draw_throughput_chart from ..labels import wrapping_label -from ..rates import average_kbps +from ..rates import average_kbps, format_rate, rate_with_unit, RATE_FIELD_KEYS, UNIT_LABEL from ..scaling import scaled from ..scrollable import ScrollableFrame from .. import scope @@ -67,6 +67,7 @@ ("loss_bursts", "stats.loss_runs", "", "tips.stat_loss_runs"), ("corrupted", "stats.corrupted", "", "tips.stat_corrupted"), ("duplicated", "stats.duplicated", "", "tips.stat_duplicated"), + ("reordered", "stats.reordered", "", "tips.stat_reordered"), ("drop_overflow", "stats.overflow", "", "tips.stat_overflow"), ("drop_shutdown", "stats.shutdown_dropped", "", "tips.stat_shutdown"), ("drop_send", "stats.send_failed", "", "tips.stat_send_failed"), @@ -134,6 +135,10 @@ def __init__(self, app, parent): self.nb.bind("<>", lambda e: self._on_subpage()) self.stat_labels = {} + self.cap_labels = {} + # Which unit the rate captions currently SAY, so the tick can rewrite them + # when the preference changes and skip the work when it has not. + self._unit_shown = None self.sess_labels = {} self._cells = [] self._grid_cols = 0 @@ -172,6 +177,7 @@ def _build_live(self, parent): style="StatCap.TLabel") caption.pack(padx=scaled(10), pady=(0, scaled(8)), anchor="w") self.stat_labels[key] = value + self.cap_labels[key] = caption for w in (cell, value, caption): add_tooltip(w, tip) self._attach_copy(w, value, "live") @@ -319,10 +325,16 @@ def session_text(self): def live_text(self): """The counter grid, same rule - `CELLS` is the source, units included.""" rows = [] + # The copied text says the same unit the screen does: this is what ends up + # pasted into a bug report, so a caption disagreeing with the number beside + # it travels further than the window it came from. + chosen = UNIT_LABEL.get(self.app.pref("rate_unit"), "") for key, cap, unit, _tip in CELLS: label = self.stat_labels.get(key) if label is None: continue + if key in RATE_FIELD_KEYS: + unit = chosen caption = T(cap) + (" (%s)" % unit if unit else "") rows.append("%s: %s" % (caption, label.cget("text"))) return "\n".join(rows) @@ -432,7 +444,8 @@ def draw_chart(self): self._chart_job = None try: draw_throughput_chart(self.canvas, self.app.down_hist, self.app.up_hist, - sample_interval_s=self.app.TICK_MS / 1000.0) + sample_interval_s=self.app.TICK_MS / 1000.0, + unit=self.app.pref("rate_unit")) except Exception as _exc: crashlog.note(_exc, "gui.pages.stats") @@ -486,19 +499,43 @@ def _sync_scope_note(self): note.config(text=T(SCOPE_NOTES[state])) retip(note, SCOPE_TIPS[state]) + def _sync_rate_captions(self, unit): + """Rewrite the "(KB/s)" in the throughput captions when the unit changes. + + On the tick rather than at build time, for the same reason the scope notes + are: the preference flips in another window while this page is already + built, and a caption naming a unit the number is no longer in is the exact + class of lie convention 5 is about. Memoised on the unit, so an unchanged + preference costs one comparison. + """ + if unit == self._unit_shown: + return + self._unit_shown = unit + label = UNIT_LABEL.get(unit, "") + with crashlog.quiet("gui.pages.stats"): + for key, cap, _unit, _tip in CELLS: + if key not in RATE_FIELD_KEYS: + continue + caption = self.cap_labels.get(key) + if caption is not None: + caption.config(text=T(cap) + (f" ({label})" if label else "")) + def refresh_counters(self): self._sync_scope_note() self._chart_frame.config(text=self._throughput_title()) snap = self.app.last_snapshot or {} rates = self.app.last_rates - self.stat_labels["down"].config(text=f"{rates[0]:.0f}") - self.stat_labels["up"].config(text=f"{rates[1]:.0f}") + unit = self.app.pref("rate_unit") + self.stat_labels["down"].config(text=format_rate(rates[0], unit)) + self.stat_labels["up"].config(text=format_rate(rates[1], unit)) + self._sync_rate_captions(unit) # `seen` is the only counter here with a scoped twin. The impairment # counters are already scoped by construction (nothing outside the target # can be impaired), and drop_overflow / drop_shutdown / drop_send stay on # the FULL traffic on purpose - they are what the TOOL lost, including # traffic the user never targeted, and narrowing them would hide it. for key in ("seen", "queue", "drop_loss", "loss_bursts", "corrupted", "duplicated", + "reordered", "drop_overflow", "drop_shutdown", "drop_send", "drop_rate", "drop_syn", "drop_mtu", "drop_nat", "drop_rst", "drop_lan", "drop_internet_only", @@ -510,6 +547,7 @@ def refresh_session(self): app = self.app snap = app.last_snapshot or {} info = app.engine.session_info() + unit = app.pref("rate_unit") host, ipv4, ipv6 = host_identity() self.sess_labels["host"].config(text=host) self.sess_labels["private_ipv4"].config(text=ipv4) @@ -538,7 +576,8 @@ def refresh_session(self): self.sess_labels["driver_wait"].config( text=f"{waited:.2f} ms" if waited else "-") self.sess_labels["peak_rate"].config( - text=f"{app.peak_down:.0f} / {app.peak_up:.0f} KB/s") + text="%s / %s" % (format_rate(app.peak_down, unit), + rate_with_unit(app.peak_up, unit))) down_mb = bytes_to_mb(app.scoped_stat(snap, "bytes_in")) up_mb = bytes_to_mb(app.scoped_stat(snap, "bytes_out")) total_mb = round(down_mb + up_mb, 2) @@ -549,7 +588,7 @@ def refresh_session(self): total_bytes = (app.scoped_stat(snap, "bytes_in") + app.scoped_stat(snap, "bytes_out")) avg = average_kbps(total_bytes, elapsed) - self.sess_labels["avg_rate"].config(text=f"{avg:.0f} KB/s") + self.sess_labels["avg_rate"].config(text=rate_with_unit(avg, unit)) def refresh_events(self): events = self.app.engine.events_snapshot()[-300:] diff --git a/beantester/gui/panels/settings.py b/beantester/gui/panels/settings.py index 282c91a..12b001a 100644 --- a/beantester/gui/panels/settings.py +++ b/beantester/gui/panels/settings.py @@ -27,7 +27,8 @@ from ..accordion import CollapsibleSection from ..form import ControlForm from ..labels import wrapping_label -from ..prefs import ACTION, BOOL, NUMBER, PREF_GROUPS, PREFS_BY_KEY, prefs_in_section +from ..prefs import (ACTION, BOOL, CHOICE, NUMBER, PREF_GROUPS, PREFS_BY_KEY, + prefs_in_section) from ..scaling import scaled from ..scrollable import ScrollableFrame from ..theme import popdown_height, unhighlight_combobox @@ -248,6 +249,36 @@ def _build_pref_row(self, card, pref): add_tooltip(btn, pref.tip) return + if pref.kind == CHOICE: + # label | readonly combobox | hint. Readonly for the same reason the + # traffic filter is: the value is one of a fixed set, and a typable box + # invites a spelling this build has no branch for. + label = ttk.Label(row, text=T(pref.label), style="Card.TLabel") + label.pack(side="left", padx=(0, scaled(6))) + labels = [text for _, text in pref.choices] + by_label = {text: value for value, text in pref.choices} + current = app.pref(pref.key) + shown = next((text for value, text in pref.choices if value == current), + labels[0]) + var = tk.StringVar(value=shown) + box = ttk.Combobox(row, textvariable=var, values=labels, + state="readonly", width=pref.width) + box.pack(side="left") + # The dropdown carries the VALUE, never the label: the labels are what + # a person reads and the values are what ui.json and every reader of + # the preference see, and collapsing the two is how a stored setting + # starts depending on the interface language. + box.bind("<>", + lambda e, k=pref.key, v=var, m=by_label: + self._store(k, m.get(v.get())), add="+") + add_tooltip(box, pref.tip) + add_tooltip(label, pref.tip) + if pref.hint: + ttk.Label(row, text=T(pref.hint), style="Hint.TLabel").pack( + side="left", padx=(scaled(8), 0)) + self._pref_vars[pref.key] = var + return + # NUMBER: label | entry | unit | hint, with live validation like the form label = ttk.Label(row, text=T(pref.label), style="Card.TLabel") label.pack(side="left", padx=(0, scaled(6))) diff --git a/beantester/gui/prefs.py b/beantester/gui/prefs.py index 803d928..83b715e 100644 --- a/beantester/gui/prefs.py +++ b/beantester/gui/prefs.py @@ -19,9 +19,12 @@ """ from typing import Any, NamedTuple, Optional, Tuple +from . import rates + NUMBER = "number" # validated float/int, inclusive bounds BOOL = "bool" # checkbox ACTION = "action" # a button that runs App.() +CHOICE = "choice" # readonly combobox over Pref.choices class Pref(NamedTuple): @@ -35,6 +38,15 @@ class Pref(NamedTuple): hint: str = "" # i18n key of the greyed hint width: int = 8 action: str = "" # App method name for kind == ACTION + # ``(stored value, label shown)`` pairs for kind == CHOICE, in the order the + # dropdown lists them. The label is a LITERAL, not an i18n key, and that is + # the same exception ``fields.Field.unit`` already carries: it is here for + # choices whose labels are SYMBOLS rather than words - "KB/s", "Mbit/s" - which + # read the same in every language and would otherwise cost three identical + # translations each, in three files, for ever. A choice whose labels are WORDS + # does not belong here: give it i18n keys instead, or it will ship English to + # every reader. Guarded by tests/test_prefs.py. + choices: Tuple[Tuple[str, str], ...] = () # Id of a ``fields.Section`` that renders this pref through its ``extra`` # builder, INSTEAD of a preference group. For the rare pref that belongs # beside a registry field rather than with the other preferences: "Show only @@ -57,6 +69,20 @@ class Pref(NamedTuple): Pref("log_lines", NUMBER, "prefs.log_lines", "tips.log_lines", default=500, bounds=(50.0, 100000.0), unit_key="prefs.unit_lines", hint="prefs.log_lines_hint", width=10), + # DISPLAY only, and that is forced rather than chosen: KB/s is what the CLI + # flags, the config file, the schedule string, the shipped scenarios and the + # NDJSON `down_kbps`/`up_kbps` fields all carry, and several of those are + # frozen contracts. So this converts on the way to the screen and never on the + # way to a file - you still TYPE a limit in KB/s, and the readout beside the + # field says what that is in the unit you picked. + # + # A Pref rather than a registry field, by convention 42: it must survive a + # restart and must NOT get a CLI flag or ride inside a traffic config, because + # a config file describes the traffic and not the window looking at it. + Pref("rate_unit", CHOICE, "prefs.rate_unit", "tips.rate_unit", + default=rates.DEFAULT_UNIT, + choices=tuple((key, label) for key, label, _ in rates.RATE_UNITS), + width=10), # Default False = what the tool has always done: every captured packet is # counted and listed, and targeting only decides what gets IMPAIRED. Turning # it on narrows the VIEW, never the capture and never the impairment - the @@ -89,7 +115,8 @@ class Pref(NamedTuple): # Prefs that name a ``section`` are rendered there instead and must NOT appear # here - see ``SECTION_PREFS`` below. PREF_GROUPS = ( - ("prefs.group_view", ("chart_seconds", "log_lines", "show_control_search")), + ("prefs.group_view", ("rate_unit", "chart_seconds", "log_lines", + "show_control_search")), ("prefs.group_behaviour", ("confirm_close", "restore_profile", "reset_layout")), ) @@ -116,4 +143,10 @@ def coerce(pref, raw): lo, hi = pref.bounds or (float("-inf"), float("inf")) value = min(max(value, lo), hi) return int(value) if float(value).is_integer() else value + if pref.kind == CHOICE: + # A value this build does not offer falls back to the default rather than + # being kept: ui.json survives downgrades and hand-editing, and a stored + # value nothing can render would leave the dropdown showing a blank while + # every reader of the preference got a string it has no branch for. + return raw if raw in {value for value, _ in pref.choices} else pref.default return raw diff --git a/beantester/gui/rates.py b/beantester/gui/rates.py index 0e9d49f..017baf8 100644 --- a/beantester/gui/rates.py +++ b/beantester/gui/rates.py @@ -24,7 +24,74 @@ anchor the window, so the span is always >= WINDOW once the session is warm. """ from collections import deque -from typing import Optional +from typing import Any, Optional + +from ..fields import FIELD_DEFS + +# -- units ------------------------------------------------------------------ # +# Everything inside this program counts throughput in KB/s, where K is 1024: the +# CLI flags, the config file, the schedule string, the shipped scenarios and the +# NDJSON `down_kbps`/`up_kbps` fields all carry that number, and several of them +# are frozen contracts. So the unit preference is a DISPLAY choice and nothing +# else - it converts on the way to the screen and never on the way to a file. +# +# 🔴 The Mbit/s factor is the one number here that surprises people, so it is +# derived in the open rather than typed: 1 KB/s is 1024 bytes, a byte is 8 bits, +# and a megabit is 1e6 bits (decimal, as every network interface and every ISP +# means it). That makes 1024 KB/s come out as 8.389 Mbit/s and NOT 8.0. Rounding +# it to 8 would be a 4.9% lie in the direction people already expect, which is +# exactly the kind of number that never gets questioned again. +BASE_LABEL = "KB/s" +RATE_UNITS = ( + ("kb", BASE_LABEL, 1.0), + ("mbit", "Mbit/s", 1024.0 * 8.0 / 1_000_000.0), + ("mb", "MB/s", 1.0 / 1024.0), +) +UNIT_FACTOR = {key: factor for key, _, factor in RATE_UNITS} +UNIT_LABEL = {key: label for key, label, _ in RATE_UNITS} +DEFAULT_UNIT = RATE_UNITS[0][0] + +# Which settings fields carry a throughput in the base unit. A VIEW over the +# field registry rather than a list of names, so a third rate field is picked up +# by declaring its unit and nothing here has to remember it. +RATE_FIELD_KEYS = tuple(f.key for f in FIELD_DEFS if f.unit == BASE_LABEL) + + +def in_unit(kbps: Any, unit: str) -> float: + """A KB/s figure expressed in ``unit``. Unknown units read as the base one. + + ``Any`` rather than ``float`` deliberately: one caller is a half-typed entry + box, so this takes whatever the widget holds and answers 0.0 for anything that + is not a number. Narrowing the annotation would push that decision out to + three call sites (see ``format_rate``). + """ + try: + value = float(kbps) + except (TypeError, ValueError): + return 0.0 + return value * UNIT_FACTOR.get(unit, 1.0) + + +def format_rate(kbps: Any, unit: str) -> str: + """``kbps`` rendered in ``unit``, with the precision that unit needs. + + KB/s keeps the whole numbers it has always printed. The other two are smaller + numbers - 1024 KB/s is 1.0 MB/s - so printing them the same way would round a + real speed limit to "0" and make the readout look broken. Three significant + figures, which is what ``utils.human_bytes`` settled on for the same reason. + """ + value = in_unit(kbps, unit) + if unit == DEFAULT_UNIT: + return f"{value:.0f}" + if value >= 100: + return f"{value:.0f}" + return f"{value:.1f}" if value >= 10 else f"{value:.2f}" + + +def rate_with_unit(kbps: Any, unit: str) -> str: + """``format_rate`` plus the unit, for the places that print both together.""" + return f"{format_rate(kbps, unit)} {UNIT_LABEL.get(unit, BASE_LABEL)}" + WINDOW_S = 1.0 # average over this much time WARMUP_S = 0.8 # below this the window is too young to trust diff --git a/beantester/repro.py b/beantester/repro.py index a9ef8b6..e2130ad 100644 --- a/beantester/repro.py +++ b/beantester/repro.py @@ -3,7 +3,7 @@ import time from .appinfo import TOOL_ID, command_name -from .engine import corruption_pct, impairment_loss_pct +from .damage import corruption_pct, impairment_loss_pct from .i18n import translate from .settings import DEFAULT_SETTINGS, setting_expression from .utils import bytes_to_mb, number_string, to_number @@ -101,6 +101,11 @@ def build_repro_report(engine, settings): # means the session was too short to see one, which is the difference # between a run that proved nothing and a tool that is broken. loss_runs=stats.get("loss_bursts", 0), + # How many packets left this tool AFTER one that arrived later than they + # did. Zero with jitter or a latency spike configured means the traffic + # was too sparse for anything to overtake anything, not that the delay + # was never applied - the same distinction loss_runs draws above. + reordered=stats.get("reordered", 0), # connections_reset held drop_rst - the PACKETS a reset connection swallows # during its cooldown, which for a 30 s cooldown on a busy flow is thousands # against a handful of actual resets. The three RST numbers answer three diff --git a/lang/en.json b/lang/en.json index 94f7b59..201f64f 100644 --- a/lang/en.json +++ b/lang/en.json @@ -205,6 +205,7 @@ "fields.period": "Period:", "fields.port": "Port:", "fields.port_label": "port:", + "fields.rate_converted": "(= {value})", "fields.row_limit": "Row limit:", "fields.row_limit_hint": "0 = no limit", "fields.rst": "TCP reset (RST):", @@ -244,7 +245,7 @@ "frames.event_log": "Event log (timestamped)", "frames.flapping": "Link outages (flapping)", "frames.impairments": "Impairments", - "frames.latency": "Latency (ping)", + "frames.latency": "Latency (ping) and packet order", "frames.profiles": "Profiles", "frames.repro": "Reproducibility & scenario", "frames.schedule": "Throughput schedule", @@ -357,6 +358,7 @@ "prefs.group_view": "Display", "prefs.log_lines": "Log lines kept", "prefs.log_lines_hint": "older lines drop off", + "prefs.rate_unit": "Speed unit:", "prefs.reset_layout": "Reset window layout", "prefs.restore_profile": "Restore the last profile on startup", "prefs.scope_view": "Show only the targeted traffic", @@ -415,6 +417,7 @@ "stats.packets": "Packets", "stats.queued": "Queued", "stats.rate_dropped": "Rate-limit drop", + "stats.reordered": "Reordered", "stats.rst_reset": "RST reset", "stats.rst_sent": "RST sent", "stats.scope_note": "Counters cover ALL captured traffic (per \"Traffic to modify\"), not only the traffic your targeting impairs.", @@ -531,6 +534,7 @@ "tips.nat": "NAT mapping expiry: if a connection is silent longer than the given seconds, the next inbound packet is dropped (the mapping 'disappears'). Tests keep-alive. 0 = off.", "tips.peak_rate": "The highest throughput reached, averaged over a 1 s window (a speed limit shapes the average rate, so a shorter window would report bursts above the limit).", "tips.profiles": "Ready-made settings (presets, e.g. '3G network') and your own. Selecting one fills the fields above immediately.", + "tips.rate_unit": "Which unit the Statistics page, the chart and the readout next to the speed limits show throughput in. You still type the limits themselves in KB/s, because that is the number a saved configuration file and the command line carry.", "tips.reset_layout": "Forget the remembered window size and position, collapsed sections and table sorting, and recentre the window. Your settings and session are kept.", "tips.reset_now": "Reset all active TCP connections now (for ~3 s). UDP has no reset - use loss or a link outage for it. Works after START.", "tips.restore_profile": "When on, the tool reopens with the last profile you picked already filled in. It does not start a capture - you still press START.", @@ -550,7 +554,7 @@ "tips.session_capture": "Which traffic the driver handed over during this session. \"Narrowed to the destination\" means the counters and the connection list cover that traffic only, because the rest never reached the tool.", "tips.settings": "App settings: interface language and how many rows the tables show.", "tips.show_control_search": "Shows the \"Search\" box at the top of the Control page. With it off the box is gone and Ctrl+F takes you to the search box in the Connections tab instead.", - "tips.spike": "Occasional ping spikes: with the given probability (%) add extra delay (ms) to a single packet. Reproduces momentary 'lag'.", + "tips.spike": "Occasional ping spikes: with the given probability (%) add extra delay (ms) to a single packet. A spiked packet arrives after ones sent later than it, so this changes packet order without making every delay wobble the way Jitter does.", "tips.start": "Turns traffic modification on/off. Picking a preset or changing fields does nothing on its own - only START begins impairing. Requires running as administrator.", "tips.stat_block": "Packets dropped by a block (firewall) rule.", "tips.stat_corrupted": "Packets whose payload had a data bit flipped. Packets with no payload (e.g. bare ACKs) can't be corrupted - they pass through and aren't counted here, so this can be below the set percentage.", @@ -566,6 +570,7 @@ "tips.stat_overflow": "Packets dropped because the queue overflowed (heavy overload). This counter always covers ALL captured traffic, even when the view is narrowed to the target: these are packets the TOOL lost, and hiding the ones outside your target would hide its own damage.", "tips.stat_queue": "How many packets are waiting in the queue (grows with latency or a speed limit).", "tips.stat_rate": "Packets dropped because the speed-limit buffer filled up (offered traffic stayed above the limit longer than the buffer holds). Counted separately from loss and from tool overflow.", + "tips.stat_reordered": "How many packets came out of the tool in a different order than they went in. Zero with Jitter or Spike chance set means the packets were too far apart for any of them to overtake another, not that the delay was missing.", "tips.stat_rst": "Packets of reset connections (RST).", "tips.stat_rst_sent": "How many RST packets were injected to reset connections.", "tips.stat_seen": "How many packets passed through the tool in total.", diff --git a/lang/pl.json b/lang/pl.json index 169d17c..97b81bb 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -205,6 +205,7 @@ "fields.period": "Okres:", "fields.port": "Port:", "fields.port_label": "port:", + "fields.rate_converted": "(= {value})", "fields.row_limit": "Limit wierszy:", "fields.row_limit_hint": "0 = bez limitu", "fields.rst": "Zrywanie TCP (RST):", @@ -244,7 +245,7 @@ "frames.event_log": "Dziennik zdarzeń (ze znacznikami czasu)", "frames.flapping": "Przerwy w łączu (flapping)", "frames.impairments": "Zakłócenia", - "frames.latency": "Opóźnienie (ping)", + "frames.latency": "Opóźnienie (ping) i kolejność pakietów", "frames.profiles": "Profile", "frames.repro": "Powtarzalność i scenariusz", "frames.schedule": "Harmonogram przepustowości", @@ -357,6 +358,7 @@ "prefs.group_view": "Wygląd", "prefs.log_lines": "Trzymane linie logu", "prefs.log_lines_hint": "starsze linie znikają", + "prefs.rate_unit": "Jednostka prędkości:", "prefs.reset_layout": "Zresetuj układ okna", "prefs.restore_profile": "Przywróć ostatni profil przy starcie", "prefs.scope_view": "Pokazuj tylko ruch celu", @@ -415,6 +417,7 @@ "stats.packets": "Pakiety", "stats.queued": "W kolejce", "stats.rate_dropped": "Odrzuc. przez limit", + "stats.reordered": "Zmieniona kolejność", "stats.rst_reset": "RST zerwane", "stats.rst_sent": "RST wysłane", "stats.scope_note": "Liczniki obejmują CAŁY przechwycony ruch (wg „Ruch do modyfikacji”), a nie tylko ten, który psuje celowanie.", @@ -531,6 +534,7 @@ "tips.nat": "Wygasanie mapowania NAT: jeśli połączenie milczy dłużej niż podana liczba sekund, kolejny pakiet przychodzący jest odrzucany (mapowanie 'znika'). Test keep-alive. 0 = wyłączone.", "tips.peak_rate": "Najwyższa osiągnięta przepustowość, uśredniona w oknie 1 s (limit prędkości kształtuje średnią, więc krótsze okno pokazywałoby skoki powyżej limitu).", "tips.profiles": "Gotowe zestawy ustawień (presety, np. 'Sieci 3G') oraz Twoje własne. Wybór od razu wypełnia pola powyżej.", + "tips.rate_unit": "Jednostka, w której Statystyki, wykres i odczyt obok limitów prędkości pokazują przepustowość. Same limity nadal wpisujesz w KB/s, bo to jest ta liczba, którą niesie zapisany plik konfiguracji i wiersz poleceń.", "tips.reset_layout": "Zapomnij zapamiętany rozmiar i pozycję okna, zwinięte sekcje i sortowanie tabel, i wyśrodkuj okno. Ustawienia i sesja zostają.", "tips.reset_now": "Zerwij teraz wszystkie aktywne połączenia TCP (na ~3 s). UDP nie ma zrywania - użyj strat albo przerwy w łączu. Działa po naciśnięciu START.", "tips.restore_profile": "Gdy włączone, narzędzie otwiera się z ostatnio wybranym profilem już wpisanym. Nie startuje przechwytywania - nadal klikasz START.", @@ -550,7 +554,7 @@ "tips.session_capture": "Jaki ruch sterownik podawał w tej sesji. „Zawężony do celu” znaczy, że liczniki i lista połączeń obejmują wyłącznie ten ruch, bo reszta w ogóle nie dotarła do narzędzia.", "tips.settings": "Ustawienia aplikacji: język interfejsu i ile wierszy pokazują tabele.", "tips.show_control_search": "Pokazuje pole „Szukaj” u góry strony Sterowanie. Po wyłączeniu pole znika, a Ctrl+F przenosi do wyszukiwarki w zakładce Połączenia.", - "tips.spike": "Sporadyczne skoki pingu: z podanym prawdopodobieństwem (%) doklej dodatkowe opóźnienie (ms) do pojedynczego pakietu. Odwzorowuje chwilowe 'lagi'.", + "tips.spike": "Sporadyczne skoki pingu: z podanym prawdopodobieństwem (%) doklej dodatkowe opóźnienie (ms) do pojedynczego pakietu. Pakiet ze skokiem dociera po tych wysłanych później niż on, więc zmienia to kolejność pakietów bez rozchwiania każdego opóźnienia, jak robi to Jitter.", "tips.start": "Włącza/wyłącza modyfikowanie ruchu. Wybór presetu lub zmiana pól sama nic nie robi - dopiero START uruchamia zakłócenia. Wymaga uruchomienia jako administrator.", "tips.stat_block": "Pakiety odrzucone przez regułę blokady (firewall).", "tips.stat_corrupted": "Pakiety, w których przekłamano bit danych. Pakietów bez danych (np. samych ACK) nie da się uszkodzić - przechodzą i nie są tu liczone, więc liczba bywa niższa niż ustawiony procent.", @@ -566,6 +570,7 @@ "tips.stat_overflow": "Pakiety porzucone, bo kolejka się przepełniła (silne przeciążenie). Ten licznik zawsze obejmuje CAŁY przechwycony ruch, nawet gdy widok jest zawężony do celu: to są pakiety zgubione przez NARZĘDZIE, a ukrycie tych spoza celu ukryłoby jego własne szkody.", "tips.stat_queue": "Ile pakietów czeka w kolejce (rośnie, gdy działa opóźnienie lub limit prędkości).", "tips.stat_rate": "Pakiety porzucone, bo bufor limitu prędkości się zapełnił (ruch powyżej limitu dłużej niż mieści bufor). Liczone osobno od strat i od przepełnienia narzędzia.", + "tips.stat_reordered": "Ile pakietów wyszło z narzędzia w innej kolejności, niż do niego weszło. Zero przy ustawionym Jitterze albo Szansie skoku znaczy, że pakiety szły zbyt rzadko, żeby któryś wyprzedził inny, a nie że opóźnienia nie było.", "tips.stat_rst": "Pakiety zerwanych połączeń (RST).", "tips.stat_rst_sent": "Ile pakietów RST wstrzyknięto, by zerwać połączenia.", "tips.stat_seen": "Ile pakietów łącznie przeszło przez narzędzie.", diff --git a/lang/zh.json b/lang/zh.json index 45dfb15..f1bd6f5 100644 --- a/lang/zh.json +++ b/lang/zh.json @@ -205,6 +205,7 @@ "fields.period": "周期:", "fields.port": "端口:", "fields.port_label": "端口:", + "fields.rate_converted": "(= {value})", "fields.row_limit": "行数上限:", "fields.row_limit_hint": "0 = 不限制", "fields.rst": "TCP 重置(RST):", @@ -244,7 +245,7 @@ "frames.event_log": "事件日志(带时间戳)", "frames.flapping": "链路中断(周期断线)", "frames.impairments": "弱网效果", - "frames.latency": "延迟(Ping)", + "frames.latency": "延迟(Ping)与数据包顺序", "frames.profiles": "配置方案", "frames.repro": "可复现性与场景", "frames.schedule": "吞吐量计划", @@ -357,6 +358,7 @@ "prefs.group_view": "显示", "prefs.log_lines": "保留的日志行数", "prefs.log_lines_hint": "更旧的日志行会被移除", + "prefs.rate_unit": "速度单位:", "prefs.reset_layout": "重置窗口布局", "prefs.restore_profile": "启动时恢复上次配置方案", "prefs.scope_view": "仅显示目标流量", @@ -415,6 +417,7 @@ "stats.packets": "数据包", "stats.queued": "队列中", "stats.rate_dropped": "因限速丢弃", + "stats.reordered": "乱序包数", "stats.rst_reset": "RST 重置", "stats.rst_sent": "已发送 RST", "stats.scope_note": "计数器覆盖所有已捕获流量(由“要修改的流量”决定),而不只是目标规则实际施加弱网的流量。", @@ -531,6 +534,7 @@ "tips.nat": "模拟 NAT 映射过期:连接静默时间超过指定秒数后,下一个入站数据包会被丢弃,相当于映射“消失”。用于测试保活机制。0 = 关闭。", "tips.peak_rate": "达到过的最高吞吐量,按 1 秒窗口取平均。限速会平滑平均速率,因此若使用更短窗口,可能看到高于限速值的瞬时突发。", "tips.profiles": "可直接使用的内置预设(例如“3G 网络”)以及你保存的配置方案。选择后会立即填入上方字段。", + "tips.rate_unit": "统计页、图表以及速度限制旁边的读数用哪种单位显示吞吐量。限速本身仍然以 KB/s 输入,因为保存的配置文件和命令行携带的就是这个数值。", "tips.reset_layout": "清除已记住的窗口大小和位置、折叠状态以及表格排序,并把窗口重新居中。你的设置和当前会话会保留。", "tips.reset_now": "立即重置全部活动 TCP 连接,持续约 3 秒。UDP 没有“重置”机制,请用丢包或链路中断模拟。启动会话后可用。", "tips.restore_profile": "启用后,工具启动时会自动填入上次选择的配置方案,但不会自动开始捕获,仍需点击“开始”。", @@ -550,7 +554,7 @@ "tips.session_capture": "本次会话中驱动实际交给工具的流量。“已收窄到目标地址”表示计数器和连接列表只覆盖该目标流量,因为其他流量根本没有进入本工具。", "tips.settings": "应用设置:界面语言以及表格最多显示的行数。", "tips.show_control_search": "控制“控制”页顶部是否显示“搜索”框。关闭后该搜索框会消失,Ctrl+F 将转到“连接”标签页中的搜索框。", - "tips.spike": "偶发 Ping 尖峰:以给定概率(%)为单个数据包额外增加指定毫秒数的延迟,用于复现瞬间卡顿。", + "tips.spike": "偶发的 Ping 尖峰:按给定概率(%)为单个数据包增加额外延迟(毫秒)。被加了尖峰的数据包会晚于比它更晚发出的包到达,因此这会改变数据包顺序,而不像抖动那样让每个包的延迟都上下波动。", "tips.start": "开启或关闭流量修改。选择预设或更改字段本身不会产生影响,只有点击“开始”才会开始施加弱网效果。需要以管理员身份运行。", "tips.stat_block": "被阻断(防火墙)规则丢弃的数据包。", "tips.stat_corrupted": "负载中有一个数据位被翻转的数据包。没有负载的数据包(例如纯 ACK)无法被损坏,会直接通过且不计入这里,因此实际比例可能低于设置值。", @@ -566,6 +570,7 @@ "tips.stat_overflow": "因队列溢出(严重过载)而被丢弃的数据包。即使视图已收窄到目标,此计数器也始终覆盖所有已捕获流量,因为这些数据包是本工具丢失的。隐藏目标之外的部分会掩盖工具自身造成的损害。", "tips.stat_queue": "当前正在队列中等待的数据包数,会随延迟或限速而增加。", "tips.stat_rate": "因限速缓冲区已满而被丢弃的数据包,即输入流量持续高于限速值且超过缓冲区承载时间。它与普通丢包及工具队列溢出分开统计。", + "tips.stat_reordered": "有多少数据包离开工具时的顺序与进入时不同。设置了抖动或尖峰概率却显示 0,说明数据包之间间隔太大,谁也没能超过谁,而不是延迟没有生效。", "tips.stat_rst": "属于已重置连接(RST)的数据包。", "tips.stat_rst_sent": "为重置连接而注入的 RST 数据包数。", "tips.stat_seen": "经过本工具的全部数据包数量。", diff --git a/tests/test_engine.py b/tests/test_engine.py index 742e482..2a36fec 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -12,7 +12,7 @@ from beantester import BeanEngine from beantester.core import BeanCore -from beantester.engine import (DROP_BY_REASON, IMPAIRMENT_DROP_KEYS, TOOL_DROP_KEYS, +from beantester.damage import (DROP_BY_REASON, IMPAIRMENT_DROP_KEYS, TOOL_DROP_KEYS, impairment_loss_pct) from beantester.settings import DEFAULT_SETTINGS, apply_settings from beantester.synthetic import SyntheticDivert diff --git a/tests/test_form_search.py b/tests/test_form_search.py index d838546..b8c8f16 100644 --- a/tests/test_form_search.py +++ b/tests/test_form_search.py @@ -141,3 +141,35 @@ def test_an_accented_label_is_reachable_without_its_accents(): check("search: and it is still reachable WITH the accents", entry.key in [e.key for e in S.find(index, entry.label)]) set_language("en") + + +# The word each language's "Latency (ping) and packet order" card uses for ORDER. +# A dict rather than one string because the guard is about the SECTION TITLE doing +# its job in the language somebody is actually reading, and Chinese does not +# contain the English word. +ORDER_WORD = {"en": "order", "pl": "kolejnosc", "zh": "顺序"} + + +def test_the_spike_pair_is_findable_by_the_effect_it_is_used_for(): + """Searching for packet ORDER must reach the spike fields, in every language. + + This is a copy-pinning test on purpose, and the exception is argued rather + than assumed. ``form_search`` matches names - field labels, section titles and + CLI flags - and deliberately NOT tooltip bodies (owner decision, 2026-08-18). + So the ONLY thing that puts these two fields in front of somebody testing a + UDP protocol against reordering is the word living in a NAME. Explaining it in + the tooltip instead is invisible to the search, and nothing would say so. + + Measured on 2026-09-04, before the section was renamed from "Latency (ping)": + zero hits for this word in both en and pl. After: the section and all four + fields inside it. Reword the card freely - just keep a word for order in it, + or this reddens and tells you what you took away. + """ + for code, word in ORDER_WORD.items(): + set_language(code) + index = S.build_index() + hits = {e.key for e in S.find(index, word)} + for key in ("spike_prob", "spike_ms"): + check(f"search[{code}]: {word!r} reaches {key}", key in hits, + f"(hits: {sorted(hits)})") + set_language("en") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 7668c25..a4d239b 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -1884,8 +1884,11 @@ # Without its own row the drop falls through to the unnamed default and # is reported as packet LOSS - the exact confusion drop_flap was split # out to end. + # Moved with DROP_BY_REASON when damage.py was carved out of engine.py + # (2026-09-04). The registry reported it the same day: a pattern that no + # longer matches is a SKIP, and a skip reads like a pass. "label": "engine: the Internet-only drop loses its own counter", - "file": "beantester/engine.py", + "file": "beantester/damage.py", "old": ' "internet_only": "drop_internet_only", "block": "drop_block",', "new": ' "block": "drop_block",', "test": "test_every_drop_counter_and_drop_reason_is_classified", @@ -2034,6 +2037,121 @@ " __import__('url' + 'lib.request')", "test": "test_a_real_run_raises_no_network_audit_event", }, + { + # The counter simply not counting. Its own tile would read 0 for ever, + # which is indistinguishable from a session where nothing overtook + # anything - the exact confusion the counter exists to end. + "label": "reordering: an overtaken packet is not counted", + "file": "beantester/engine.py", + "old": ' self._bump("reordered")', + "new": " pass", + "test": "test_a_packet_that_is_overtaken_is_counted_as_reordered", + }, + { + # The other direction, and the one a bare "does it count?" test misses: + # a counter that increments per delivered packet passes the test above + # and means nothing. + "label": "reordering: every packet counts as reordered", + "file": "beantester/engine.py", + "old": " if arrived < self._last_sent[is_out]:", + "new": " if True:", + "test": "test_packets_that_keep_their_order_are_not_counted", + }, + { + # One shared high-water mark instead of one per direction. Ordinary + # two-way traffic then ticks the counter for inbound and outbound + # packets overtaking each other, which no receiver can observe. + "label": "reordering: one high-water mark shared by both directions", + "file": "beantester/engine.py", + "old": " if arrived < self._last_sent[is_out]:\n" + ' self._bump("reordered")\n' + " else:\n" + " self._last_sent[is_out] = arrived", + "new": " if arrived < self._last_sent[True]:\n" + ' self._bump("reordered")\n' + " else:\n" + " self._last_sent[True] = arrived", + "test": "test_the_two_directions_are_judged_separately", + }, + { + # Marking a packet as sent on the FAILURE path. The packet behind it is + # then judged against one that never reached the stack and reported as + # overtaken by it. Surgical on purpose: it touches only the except + # branch, so the other four tests here stay green and this one is shown + # to be load-bearing on its own. + "label": "reordering: a refused send still moves the mark", + "file": "beantester/engine.py", + "old": ' self._bump("drop_send")\n' + ' self._charge_flow(key, "dropped")\n', + "new": ' self._bump("drop_send")\n' + ' self._charge_flow(key, "dropped")\n' + " self._note_order(arrived,\n" + ' bool(getattr(packet, "is_outbound", True)))\n', + "test": "test_a_packet_the_driver_refused_does_not_make_the_next_one_look_overtaken", + }, + { + # The mark surviving a restart while the counter is zeroed. The next + # session then reports reordering it never did, on its very first + # packets, and the numbers look plausible. + "label": "reordering: a restart keeps the previous high-water mark", + "file": "beantester/engine.py", + "old": " self._last_sent = {True: -1, False: -1}", + "new": ' self._last_sent = getattr(self, "_last_sent", {True: -1, False: -1})', + "test": "test_a_restarted_session_does_not_inherit_the_previous_high_water_mark", + }, + { + # The comfortable answer: a megabit as 1024*1024 bits makes 1024 KB/s come + # out as exactly 8.00, which is the number a reader expects and is 4.9% + # wrong. Nothing but an assertion on the digits can catch it, because the + # wrong version looks MORE right than the correct one. + "label": "units: a megabit becomes binary, so 1024 KB/s reads 8.00", + "file": "beantester/gui/rates.py", + "old": ' ("mbit", "Mbit/s", 1024.0 * 8.0 / 1_000_000.0),', + "new": ' ("mbit", "Mbit/s", 1024.0 * 8.0 / 1_048_576.0),', + "test": "test_a_kilobyte_here_is_1024_bytes_and_a_megabit_is_a_million_bits", + }, + { + # Writing the LABEL into ui.json instead of the value. It survives the + # restart, matches no known unit, falls back to KB/s - and reads as "the + # preference does not stick" rather than as a bug in the write. + "label": "units: the dropdown stores its label instead of its value", + "file": "beantester/gui/panels/settings.py", + "old": " self._store(k, m.get(v.get())), add=\"+\")", + "new": " self._store(k, v.get()), add=\"+\")", + "test": "test_the_dropdown_stores_the_value_and_never_the_label", + }, + { + # The view over the registry replaced by a list of names - the drift this + # project keeps paying for, and invisible until a third rate field exists. + "label": "units: the rate fields become a hand-written list", + "file": "beantester/gui/rates.py", + "old": "RATE_FIELD_KEYS = tuple(f.key for f in FIELD_DEFS if f.unit == BASE_LABEL)", + "new": 'RATE_FIELD_KEYS = ("down",)', + "test": "test_the_rate_fields_are_a_view_over_the_registry_not_a_list_of_names", + }, + { + # "1024 KB/s" printed beside a box that says 1024. Harmless-looking, and + # the reason the readout exists at all is that it says something the box + # does not. + "label": "units: the converted readout repeats the value in the base unit", + "file": "beantester/gui/form.py", + "old": " if unit == DEFAULT_UNIT or var is None:", + "new": " if var is None:", + "test": "test_the_converted_readout_appears_only_when_there_is_something_to_convert", + }, + { + # The WIRING rather than the label: the page stops reacting to the + # preference, so the readout keeps naming the unit you just changed away + # from until something else rebuilds the form. Worth its own entry because + # this reaction has already moved once (out of App.set_pref, which sits on + # the size ratchet) and the test had to be pointed at the real path before + # it could see the difference. + "label": "units: the Control page stops reacting to the unit preference", + "file": "beantester/gui/pages/control.py", + "old": " self.form.sync_rate_hints()", + "new": " pass", + "test": "test_the_converted_readout_appears_only_when_there_is_something_to_convert", + }, ] # The runner's own check: a patch that cannot compile must be reported as BROKEN, not diff --git a/tests/test_rate_units.py b/tests/test_rate_units.py new file mode 100644 index 0000000..b06dec7 --- /dev/null +++ b/tests/test_rate_units.py @@ -0,0 +1,190 @@ +"""The speed-unit preference: a DISPLAY choice, and it may never become more. + +Why this file exists separately from ``test_prefs.py``: the preference is the +small half. The load-bearing half is that KB/s stays the only unit anything +DURABLE ever sees - the CLI flags, the config file, the schedule string, the +shipped scenarios and the NDJSON ``down_kbps``/``up_kbps`` fields all carry that +number, and several of them are frozen contracts. A unit switch that leaked into +any of them would silently rewrite what a saved file means, and the file would +still load. + +So the tests below come in two halves: the arithmetic (pure, and the Mbit/s +factor is the one number here people get wrong), and the boundary (what the +switch is not allowed to touch). +""" +from beantester import fields as F +from beantester.gui import rates +from beantester.gui.prefs import CHOICE, PREFS, coerce +from fakes import check +from gui_harness import run_gui + + +def test_a_kilobyte_here_is_1024_bytes_and_a_megabit_is_a_million_bits(): + """The conversion, with the number people expect written down as wrong. + + 1024 KB/s is 8.389 Mbit/s and NOT 8.0. K is 1024 in this program (the engine + multiplies by 1024 to get bytes per second), a byte is 8 bits, and a megabit + is a decimal million - which is what every network interface and every ISP + means by it. Rounding the answer to a comfortable 8 would be a 4.9% error in + exactly the direction a reader already expects, which is the kind of number + that never gets questioned again. + """ + check("1024 KB/s is 1.00 MB/s", rates.rate_with_unit(1024, "mb") == "1.00 MB/s", + f"({rates.rate_with_unit(1024, 'mb')})") + check("1024 KB/s is 8.39 Mbit/s, not 8.00", + rates.rate_with_unit(1024, "mbit") == "8.39 Mbit/s", + f"({rates.rate_with_unit(1024, 'mbit')})") + exact = 1024 * 1024 * 8 / 1_000_000 + check("and the factor is derived, not typed", + abs(rates.in_unit(1024, "mbit") - exact) < 1e-9, + f"({rates.in_unit(1024, 'mbit')} vs {exact})") + check("the base unit is a no-op", rates.in_unit(937, "kb") == 937) + check("an unknown unit reads as the base one rather than raising", + rates.in_unit(937, "furlongs per fortnight") == 937) + + +def test_the_small_units_keep_the_digits_the_base_unit_does_not_need(): + """256 KB/s must not print as "0 MB/s". + + KB/s has always printed whole numbers and still does. The other two are + smaller numbers for the same speed, so printing them the same way would round + a real, configured limit to zero and make the readout look broken rather than + small. + """ + check("256 KB/s is 0.25 MB/s", rates.format_rate(256, "mb") == "0.25", + f"({rates.format_rate(256, 'mb')})") + check("...and the base unit is still an integer", + rates.format_rate(256, "kb") == "256", f"({rates.format_rate(256, 'kb')})") + check("a big number drops the decimals rather than reading like a measurement", + rates.format_rate(1024 * 200, "mbit") == "1678", + f"({rates.format_rate(1024 * 200, 'mbit')})") + check("garbage in is 0, not an exception (this runs on a half-typed field)", + rates.format_rate("12x", "mbit") == "0.00") + + +def test_the_rate_fields_are_a_view_over_the_registry_not_a_list_of_names(): + """A third rate field must arrive here by declaring its unit, not by editing. + + The moment this becomes a hand-written tuple it starts drifting, which is the + failure mode this project keeps paying for. Derived means a new field with + ``unit="KB/s"`` gets its converted readout with no second edit. + """ + expected = tuple(f.key for f in F.FIELD_DEFS if f.unit == rates.BASE_LABEL) + check("the rate fields come out of the field registry", + rates.RATE_FIELD_KEYS == expected, + f"({rates.RATE_FIELD_KEYS} vs {expected})") + check("and today that is download and upload", + set(rates.RATE_FIELD_KEYS) == {"down", "up"}, + f"({rates.RATE_FIELD_KEYS})") + + +def test_every_choice_preference_defaults_to_one_of_its_own_choices(): + """A default outside the list renders as a blank dropdown and stores nothing.""" + choices = [p for p in PREFS if p.kind == CHOICE] + check("there is at least one CHOICE pref (else this test is vacuous)", choices) + for pref in choices: + values = [value for value, _ in pref.choices] + check(f"{pref.key}: the default is one of its choices", + pref.default in values, f"({pref.default!r} not in {values})") + check(f"{pref.key}: an unknown stored value falls back to the default", + coerce(pref, "something-a-later-build-removed") == pref.default) + check(f"{pref.key}: a known one survives", + coerce(pref, values[-1]) == values[-1]) + + +def test_the_unit_preference_never_reaches_the_engine_or_a_file(): + """The boundary, and the reason the switch is a Pref rather than a field. + + A registry field would get a CLI flag and ride inside a saved traffic config + (convention 42), so a config written with Mbit/s selected would carry a + display choice into a file that describes TRAFFIC - and the schedule string + beside it would still be in KB/s. This asserts the separation mechanically + instead of trusting that nobody moves it later. + """ + check("the unit is not a settings field", + "rate_unit" not in {f.key for f in F.FIELD_DEFS}) + from beantester.settings import DEFAULT_SETTINGS + check("...and not a settings key", "rate_unit" not in DEFAULT_SETTINGS) + from beantester.cli import build_arg_parser + flags = {a for action in build_arg_parser()._actions + for a in action.option_strings} + check("...and has no CLI flag", "--rate-unit" not in flags, f"({sorted(flags)[:3]}...)") + check("the rate FIELDS still declare the base unit, whatever is on screen", + all(F.FIELDS[k].unit == rates.BASE_LABEL for k in rates.RATE_FIELD_KEYS)) + + +def test_the_dropdown_stores_the_value_and_never_the_label(): + """The bug this kind invites: writing "Mbit/s" into ui.json instead of "mbit". + + The combobox shows labels and the store keeps values, so the two are one + mapping apart - and a label written to disk would survive a restart, fail to + match any known unit and silently fall back to KB/s, which reads as "the + preference does not stick" rather than as a bug in the write. + """ + run_gui(""" + panel = app.open_window("settings") + var = panel._pref_vars["rate_unit"] + + # Find the combobox this pref rendered and fire ITS binding, so the + # label -> value mapping under test is the production one rather than a + # copy of it written here. + from fake_tk import walk + boxes = [w for w in walk(panel.win) + if getattr(w, "kw", {}).get("textvariable") is var] + assert boxes, "the choice pref rendered no widget bound to its variable" + box = boxes[0] + assert list(box.kw.get("values")) == ["KB/s", "Mbit/s", "MB/s"], box.kw.get("values") + assert box.kw.get("state") == "readonly", box.kw.get("state") + + var.set("Mbit/s") + for handler in box.bindings.get("<>", []): + handler(None) + + assert app.pref("rate_unit") == "mbit", app.pref("rate_unit") + assert app.ui.get("pref.rate_unit") == "mbit", app.ui.get("pref.rate_unit") + """) + + +def test_the_converted_readout_appears_only_when_there_is_something_to_convert(): + """Beside the speed limits: empty in KB/s, filled otherwise, quiet on garbage. + + Repeating "1024 KB/s" next to a box that says 1024 is noise, so the base unit + shows nothing. A half-typed value shows nothing either: this readout is a + comment on the box, not a validator, and colouring the field or logging from + here would make it one. + """ + run_gui(""" + from beantester.gui.pages import pref_changed + + app.select_page("control") + form = app.form + assert set(form.rate_hints) == {"down", "up"}, sorted(form.rate_hints) + + def pick(unit): + # The production path: the Settings window persists and then TELLS the + # pages (SettingsWindow._store -> pages.pref_changed). Calling + # form.sync_rate_hints() directly here would test the label and leave + # the WIRING unguarded - which is the half that broke when the reaction + # moved out of App.set_pref. + app.set_pref("rate_unit", unit) + pref_changed(app, "rate_unit") + + app.vars["down"].set("1024") + pick("kb") + assert form.rate_hints["down"].kw.get("text") == "", \\ + form.rate_hints["down"].kw.get("text") + + pick("mbit") + shown = form.rate_hints["down"].kw.get("text") + assert "8.39" in shown and "Mbit/s" in shown, shown + + pick("mb") + shown = form.rate_hints["down"].kw.get("text") + assert "1.00" in shown and "MB/s" in shown, shown + + # a value in the middle of being typed says nothing rather than erroring + app.vars["down"].set("12x") + form.sync_rate_hints() + assert form.rate_hints["down"].kw.get("text") == "", \\ + form.rate_hints["down"].kw.get("text") + """) diff --git a/tests/test_reordering.py b/tests/test_reordering.py new file mode 100644 index 0000000..6ef6db4 --- /dev/null +++ b/tests/test_reordering.py @@ -0,0 +1,174 @@ +"""The ``reordered`` counter: did the tool actually change the order of packets? + +Why this counter needs a guard of its own. Configuring jitter or a latency spike +does NOT mean anything was reordered - whether a delayed packet is overtaken +depends on how far apart the packets were, which is a property of the traffic +rather than of the settings. So a run where nothing ever overtook anything reads +exactly like a run where the application coped, and the only thing that can tell +them apart is a number. That is the same hole ``loss_bursts`` was added to close +for losses arriving in runs. + +These tests drive ``BeanEngine._enqueue`` directly rather than going through +``decide()``. That is deliberate: the question here is what the INJECTOR counts +given a known release order, and driving the decision pipeline would make the +test depend on a random draw to produce the very ordering it wants to assert on. +The decision side is covered by ``test_core.py``. + +The release offsets below are generous (hundreds of ms for the "late" packet) +for one reason worth stating: the heap pops by release time, so the ORDER under +test is deterministic no matter how loaded the machine is - the single way this +could go wrong is the injector sending the late packet before the early one has +been queued at all, which needs a stall longer than that offset between two +adjacent statements. +""" +import time + +from beantester import BeanEngine + +from fakes import FakeDivert, FakePacket, check + + +class RefusingDivert(FakeDivert): + """A diverter that refuses ONE packet, so a failed send can be observed.""" + + def __init__(self, packets, refuse): + super().__init__(packets) + self.refuse = refuse + + def send(self, p, recalculate_checksum=True): + if p is self.refuse: + raise OSError("the driver refused this one") + super().send(p, recalculate_checksum=recalculate_checksum) + + +def _run(releases, divert=None): + """Queue ``(offset, packet)`` pairs in order, wait for delivery, return stats. + + ``releases`` is queued in list order, so the first entry is the packet that + ARRIVED first - which is the whole variable these tests turn. + """ + fake = divert if divert is not None else FakeDivert([]) + engine = BeanEngine() + engine.start("test", divert=fake) + try: + now = time.monotonic() + for offset, packet in releases: + engine._enqueue(now + offset, packet) + deadline = time.time() + 10 + while time.time() < deadline: + s = engine.stats_snapshot() + if s["queue"] == 0 and len(fake.sent) + s["drop_send"] >= len(releases): + break + time.sleep(0.01) + time.sleep(0.05) + return engine.stats_snapshot(), [p for _, p in fake.sent] + finally: + engine.stop() + + +def test_a_packet_that_is_overtaken_is_counted_as_reordered(): + """The point of the counter: one packet leaves after one that arrived later.""" + first = FakePacket(port=1001) + second = FakePacket(port=1002) + stats, sent = _run([(0.40, first), (0.05, second)]) + + check("the packet that arrived second was sent first", + sent == [second, first], f"(sent {len(sent)} packets)") + check("one reorder is counted", stats["reordered"] == 1, + f"(reordered={stats['reordered']})") + + +def test_packets_that_keep_their_order_are_not_counted(): + """The other half, and the one that stops the counter being always-on. + + Without this, a counter that simply incremented per packet would pass the + test above and be worthless. + """ + first = FakePacket(port=1001) + second = FakePacket(port=1002) + stats, sent = _run([(0.05, first), (0.30, second)]) + + check("the order was kept", sent == [first, second], f"(sent {len(sent)})") + check("nothing is counted as reordered", stats["reordered"] == 0, + f"(reordered={stats['reordered']})") + + +def test_the_two_directions_are_judged_separately(): + """A design decision, pinned: the high-water mark is PER DIRECTION. + + An inbound packet overtaking an outbound one is not a reorder anybody can + observe - they are different conversations. With one shared mark this reads + as a reorder, so the mistake would be invisible except as a counter that + ticks up on ordinary two-way traffic. + """ + outbound = FakePacket(port=1001, is_outbound=True) + inbound = FakePacket(port=1002, is_outbound=False) + stats, sent = _run([(0.40, outbound), (0.05, inbound)]) + + check("the inbound packet went out first", sent == [inbound, outbound], + f"(sent {len(sent)})") + check("crossing directions is not a reorder", stats["reordered"] == 0, + f"(reordered={stats['reordered']})") + + +def test_a_packet_the_driver_refused_does_not_make_the_next_one_look_overtaken(): + """Counted after send(), not before - and this is what makes the difference. + + The refused packet arrived LAST and was released FIRST. If the counter marked + it as sent before the driver had taken it, the packet behind it would be + judged against a packet that never reached the stack and reported as + overtaken by it. + """ + arrived_first = FakePacket(port=1001) + arrived_second = FakePacket(port=1002) + fake = RefusingDivert([], refuse=arrived_second) + stats, sent = _run([(0.40, arrived_first), (0.05, arrived_second)], divert=fake) + + check("only the accepted packet was sent", sent == [arrived_first], + f"(sent {len(sent)})") + check("the refused packet was counted as a failed send", + stats["drop_send"] == 1, f"(drop_send={stats['drop_send']})") + check("nothing is reported as overtaken", stats["reordered"] == 0, + f"(reordered={stats['reordered']})") + + +def test_a_restarted_session_does_not_inherit_the_previous_high_water_mark(): + """Second sessions start clean, or every early packet reads as overtaken. + + ``reset_stats`` zeroes the counter itself; the mark the counter is judged + against has to go with it. Missing that, the numbering carries on across the + restart while the mark stays high, so the next session reports reordering it + never did. + """ + engine = BeanEngine() + + first_run = FakeDivert([]) + engine.start("test", divert=first_run) + now = time.monotonic() + engine._enqueue(now + 0.40, FakePacket(port=1001)) + engine._enqueue(now + 0.05, FakePacket(port=1002)) + deadline = time.time() + 10 + while time.time() < deadline and len(first_run.sent) < 2: + time.sleep(0.01) + check("the first session did reorder", engine.stats_snapshot()["reordered"] == 1, + f"(reordered={engine.stats_snapshot()['reordered']})") + engine.stop() + + second_run = FakeDivert([]) + engine.start("test", divert=second_run) + try: + now = time.monotonic() + engine._enqueue(now + 0.05, FakePacket(port=2001)) + engine._enqueue(now + 0.30, FakePacket(port=2002)) + deadline = time.time() + 10 + while time.time() < deadline and len(second_run.sent) < 2: + time.sleep(0.01) + time.sleep(0.05) + stats = engine.stats_snapshot() + finally: + engine.stop() + + check("the second session sent two packets in order", + len(second_run.sent) == 2, f"(sent {len(second_run.sent)})") + check("the second session reports no reordering", stats["reordered"] == 0, + f"(reordered={stats['reordered']})")