diff --git a/CHANGELOG.md b/CHANGELOG.md index b78ff67..9ecfaf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ The format follows [Keep a Changelog](https://keepachangelog.com/); versions fol ### Fixed +- **100% loss with "Losses in a row" set now loses every packet.** A run length used to + let 11 to 33% of packets through, including during the full outage in the + `mobile-lte-to-3g` scenario, and the log and the summary described runs of loss that + did not exist. A saved "Reproduce:" command with this combination now repeats a run + that loses every packet. + +- **With Asymmetry on, the upload's runs of loss are described too.** The log now says + when the upload loss cannot reach the number you set in runs that short, and how far + apart its runs will be, and the summary shows the upload's run length. Before, only + the download was described. + - **Patterns with a repeat inside a repeat are refused as too slow.** Patterns such as `re:^(\w+)+$` used to pass the speed check, then took seconds on a single name or address and could stall the network. They are now refused when you type them. A diff --git a/README.md b/README.md index ba12dc6..d369a00 100644 --- a/README.md +++ b/README.md @@ -384,7 +384,8 @@ Two limits worth knowing, and the log states both when you apply the settings. A cannot arrive in very short runs, because runs that short leave too little room between them, so the run says what it will really deliver. And a long run length puts the runs far apart, so a short session may not see one at all. Each direction gets its own runs, so a run of twenty means twenty -in a row in that direction. +in a row in that direction, and with Asymmetry on the log states both limits for the upload too. +At 100% loss every packet is lost, so the run length changes nothing. **Link flapping** - cyclic total loss of traffic: every *Period* seconds the link is dead for the given percentage of the time. Simulates a flickering connection. This is **not** the same as losses @@ -1029,7 +1030,7 @@ what `packets_seen` counted in the first place - so every row records it in `cap | `packets_seen` | packets captured | | `packets_in_scope` | of those, the ones targeting selected for impairment | | `dropped_loss` | dropped by the Loss setting | -| `loss_runs` | how many RUNS that loss arrived in (see "Losses in a row"). 0 with a run length set means the session was too short to see one | +| `loss_runs` | how many RUNS that loss arrived in (see "Losses in a row"). 0 with a run length set means the session was too short to see one, or the loss is 100% and there are no separate runs | | `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 | diff --git a/beantester/core.py b/beantester/core.py index a92478f..4293c55 100644 --- a/beantester/core.py +++ b/beantester/core.py @@ -43,7 +43,8 @@ def burst_loss_params(loss, mean_burst): Returns ``(p, r, achievable)`` - the good-to-bad and bad-to-good transition probabilities, plus the loss fraction that pair actually delivers - or ``None`` when the loss should stay INDEPENDENT, which is the behaviour that - predates this function and the one every default still takes. + predates this function and the one every default still takes. Total loss is + one of those: every packet goes, so there is no run length to deliver. The model is Gilbert's two-state burst-noise channel (Gilbert 1960, extended by Elliott 1963), the same one ``tc netem`` offers as ``loss gemodel``. It is @@ -100,10 +101,14 @@ def burst_loss_params(loss, mean_burst): r = 1.0 / mean_burst room = 1.0 - loss if room <= 0.0: - # Total loss. Every packet goes whatever the chain says, so the chain may - # as well stay bad - and this branch is what keeps the division below - # from raising on exactly this input. - return (1.0, r, 1.0) + # Total loss has no runs to shape: there is no gap for one to end in. The + # chain cannot say that - with r > 0 it leaves the bad state, and the packet + # it leaves on goes THROUGH. Measured before this was fixed: 100% asked for + # in runs of 2, 4, 6 and 8 delivered 66.6, 79.9, 85.6 and 88.9% (external + # review, P1-1). The independent draw at 1.0 drops every packet with the + # same ONE draw per packet the chain makes, so a seed replays the same + # way. This branch is also what keeps the division below from raising. + return None p = loss * r / room if p >= 1.0: # More loss than runs this short can carry. The good state then lasts a diff --git a/beantester/settings.py b/beantester/settings.py index 227641c..66c7069 100644 --- a/beantester/settings.py +++ b/beantester/settings.py @@ -525,7 +525,13 @@ def _destination_is_frozen(engine, dst_ip, dst_port): or str(getattr(core, "dst_port", "")) != str(dst_port)) -def _say_what_the_burst_loss_will_do(loss_pct, mean_burst, log): +# The clamp line and the gap line, once per direction that has a loss of its own: +# the download (both ways with asymmetry off), then the upload. +_BURST_LINES_DOWN = ("log.loss_burst_clamped", "log.loss_burst_gap") +_BURST_LINES_UP = ("log.loss_burst_clamped_up", "log.loss_burst_gap_up") + + +def _say_what_the_burst_loss_will_do(g, log): """Two things a person cannot read off the two fields in front of them. Said at APPLY time, before the run, because both of them are the difference @@ -543,20 +549,40 @@ def _say_what_the_burst_loss_will_do(loss_pct, mean_burst, log): look reasonable and nothing happens, which reads exactly like a broken tool. So the run length AND the expected distance between runs are said out loud, in packets, which is the unit the field is in. + + Total loss says neither: every packet goes, so there are no runs to describe + (``burst_loss_params`` answers None), the same silence as a run length with no + loss at all. + + Per DIRECTION, which is the half that was missing (external review, P3-4). + The run length is one field (ADR 2026-09-01), but each direction walks its own + chain derived from its own loss, so an upload loss the runs cannot carry is + clamped exactly like a download one - and was, in silence. The upload is said + only with asymmetry on: off, its values are not read at all, and a line about + them would describe a link this session is not producing. A function of its + own so ``apply_settings`` gains no branch. """ + _say_burst_loss_for(g("loss"), g("loss_burst"), _BURST_LINES_DOWN, log) + if g("asym"): + _say_burst_loss_for(g("loss_up"), g("loss_burst"), _BURST_LINES_UP, log) + + +def _say_burst_loss_for(loss_pct, mean_burst, keys, log): + """One direction's two lines; ``keys`` names its clamp line and its gap line.""" loss = to_number(loss_pct) / 100.0 params = burst_loss_params(loss, to_number(mean_burst)) if params is None: return + clamped_key, gap_key = keys _p, _r, achievable = params if achievable < loss: - log(T("log.loss_burst_clamped", burst=number_string(mean_burst), + log(T(clamped_key, burst=number_string(mean_burst), asked=number_string(loss_pct), delivered=number_string(round(achievable * 100.0, 2)))) # Packets per cycle: one run of `mean_burst` for every `mean_burst/achievable` # packets that go past. Rounded to whole packets - the field is in packets and # a fractional one would read as precision this cannot have. - log(T("log.loss_burst_gap", burst=number_string(mean_burst), + log(T(gap_key, burst=number_string(mean_burst), gap=number_string(round(to_number(mean_burst) / achievable)))) @@ -595,7 +621,7 @@ def apply_settings(engine, s, log=lambda *_: None): # assignments and nothing else. The log lines come out in the order they # always did - only their interleaving with the setters is gone, and setters # do not log. - _say_what_the_burst_loss_will_do(g("loss"), g("loss_burst"), log) + _say_what_the_burst_loss_will_do(g, log) dst_ip = setting_expression("dst_ip", g("dst_ip")) dst_port = setting_expression("dst_port", g("dst_port")) # With the driver filter narrowed, the destination fields are START-ONLY, and diff --git a/beantester/summary.py b/beantester/summary.py index 41d4b98..0679869 100644 --- a/beantester/summary.py +++ b/beantester/summary.py @@ -5,6 +5,30 @@ from .utils import number_string, to_number +def _plain_parts(g, tr, num, pairs): + """One phrase per ``(key, phrase)`` pair whose value is not zero.""" + return [tr(phrase, v=num(key)) for key, phrase in pairs if to_number(g(key))] + + +def _loss_parts(g, tr, num, key): + """The upload's loss, with its run length right after it. + + The same question ``settings_summary`` asks inline for the download (it says + there why it is not shared): the run length is one field, but whether it + SHAPES a direction depends on that direction's loss. It is asked of the + function that decides it rather than compared against a threshold here, so + the strip cannot claim runs the engine is not producing - at 100% loss, for + one, there are none. + """ + if not to_number(g(key)): + return [] + parts = [tr("summary.loss", v=num(key))] + if burst_loss_params(to_number(g(key)) / 100.0, + to_number(g("loss_burst"))) is not None: + parts.append(tr("summary.loss_burst", v=num("loss_burst"))) + return parts + + def _upload_parts(g, tr, num): """The upload half of the description, or nothing when the link is symmetric. @@ -15,17 +39,18 @@ def _upload_parts(g, tr, num): An asymmetric run with every upload value at zero still says so. Silence there would be the misleading answer: the reader would take the numbers above to apply in both directions, which is exactly what they no longer do. + + In the download half's order, run length included: until the external review + (P3-4) the upload loss was said without it, so an upload losing in runs read + exactly like one losing evenly. """ if not g("asym"): return [] - inner = [] - for key, phrase in (("latency_up", "summary.latency"), - ("jitter_up", "summary.jitter"), - ("loss_up", "summary.loss"), - ("corrupt_up", "summary.corrupt"), - ("dup_up", "summary.dup")): - if to_number(g(key)): - inner.append(tr(phrase, v=num(key))) + inner = (_plain_parts(g, tr, num, (("latency_up", "summary.latency"), + ("jitter_up", "summary.jitter"))) + + _loss_parts(g, tr, num, "loss_up") + + _plain_parts(g, tr, num, (("corrupt_up", "summary.corrupt"), + ("dup_up", "summary.dup")))) if to_number(g("spike_prob_up")) and to_number(g("spike_ms_up")): inner.append(tr("summary.spikes", ms=num("spike_ms_up"), p=num("spike_prob_up"))) @@ -57,6 +82,11 @@ def settings_summary(s, lang=None, prefix_key="summary.prefix"): # Same loss figure, very different link: the run length is asked of the # function that DECIDES it rather than compared against a threshold here, # so the strip cannot claim runs the engine is not producing. + # 🔴 Inline, not _loss_parts, and that is about the complexity ratchet, not + # taste: this function IS `max-complexity` in pyproject.toml. Moving these + # two branches out lowers the ceiling onto `decide` - which leaves the + # "add an impairment" recipe no room - and doubles COMPLEX_NEAR_CEILING + # (measured 2026-09-29: 4 -> 8). Both are the owner's call, not a tidy-up's. if burst_loss_params(to_number(g("loss")) / 100.0, to_number(g("loss_burst"))) is not None: parts.append(tr("summary.loss_burst", v=num("loss_burst"))) diff --git a/lang/en.json b/lang/en.json index 10fc2bd..d06a3e1 100644 --- a/lang/en.json +++ b/lang/en.json @@ -303,7 +303,9 @@ "log.loaded_profile": "Loaded profile", "log.loop": "loop", "log.loss_burst_clamped": "Loss cannot reach {asked}% in runs of {burst} packets, so this session will lose {delivered}%.", + "log.loss_burst_clamped_up": "Upload loss cannot reach {asked}% in runs of {burst} packets, so uploads in this session will lose {delivered}%.", "log.loss_burst_gap": "Loss will arrive in runs of about {burst} packets, roughly one run every {gap} packets.", + "log.loss_burst_gap_up": "Upload loss will arrive in runs of about {burst} packets, roughly one run every {gap} packets.", "log.marker_needs_start": "Bug marker works after start (START).", "log.narrow_applied": "Capturing only the targeted traffic - the driver hands over your destination's traffic and nothing else, so the counters and the connection list cover that traffic only.", "log.narrow_no_effect": "\"Capture only the targeted traffic\" had no effect - this destination cannot be turned into a driver filter (a wildcard, an re: pattern, or no destination at all). Capturing everything, as usual.", @@ -590,7 +592,7 @@ "tips.stat_lan": "Packets to/from the internet dropped in LAN mode.", "tips.stat_local": "Packets to/from the local network dropped by \"Internet only\".", "tips.stat_loss": "Packets dropped because of the configured Loss. Link outages are counted separately, under Link outage.", - "tips.stat_loss_runs": "How many runs of lost packets this session produced. Zero with \"Losses in a row\" set means the session was too short to see one, not that nothing was configured.", + "tips.stat_loss_runs": "How many runs of lost packets this session produced. Zero with \"Losses in a row\" set means the session was too short to see one, not that nothing was configured. At 100% loss every packet is lost, so there are no separate runs to count.", "tips.stat_mtu": "Packets dropped as too large (MTU black hole).", "tips.stat_nat": "Packets dropped after the NAT mapping expired.", "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.", diff --git a/lang/pl.json b/lang/pl.json index b4f5da8..1fbba3d 100644 --- a/lang/pl.json +++ b/lang/pl.json @@ -303,7 +303,9 @@ "log.loaded_profile": "Wczytano profil", "log.loop": "pętla", "log.loss_burst_clamped": "Strata nie osiągnie {asked}% przy seriach po {burst} pakietów, więc ta sesja zgubi {delivered}%.", + "log.loss_burst_clamped_up": "Strata przy wysyłaniu nie osiągnie {asked}% w seriach po {burst} pakietów, więc wysyłanie w tej sesji zgubi {delivered}%.", "log.loss_burst_gap": "Strata będzie przychodzić seriami po około {burst} pakietów, mniej więcej jedna seria na {gap} pakietów.", + "log.loss_burst_gap_up": "Strata przy wysyłaniu będzie przychodzić seriami po około {burst} pakietów, mniej więcej jedna seria na {gap} pakietów.", "log.marker_needs_start": "Znacznik błędu zadziała po uruchomieniu (START).", "log.narrow_applied": "Przechwytywanie tylko ruchu celu - sterownik podaje ruch z Twoim celem i nic więcej, więc liczniki i lista połączeń obejmują wyłącznie ten ruch.", "log.narrow_no_effect": "„Przechwytuj tylko ruch celu” nic nie dało - tego celu nie da się zamienić na filtr sterownika (wildcard, wzorzec re: albo brak celu). Przechwytywane jest wszystko, jak zwykle.", @@ -590,7 +592,7 @@ "tips.stat_lan": "Pakiety do/od internetu odrzucone w trybie LAN.", "tips.stat_local": "Pakiety do/od sieci lokalnej odrzucone przez „Tylko internet”.", "tips.stat_loss": "Pakiety porzucone z powodu ustawionej Utraty. Przerwy w łączu mają własny licznik - Przerwa w łączu.", - "tips.stat_loss_runs": "Ile serii gubionych pakietów wyszło w tej sesji. Zero przy ustawionym polu \"Straty pod rząd\" znaczy, że sesja była za krótka, żeby zobaczyć choć jedną, a nie że nic nie było ustawione.", + "tips.stat_loss_runs": "Ile serii gubionych pakietów wyszło w tej sesji. Zero przy ustawionym polu \"Straty pod rząd\" znaczy, że sesja była za krótka, żeby zobaczyć choć jedną, a nie że nic nie było ustawione. Przy stracie 100% ginie każdy pakiet, więc nie ma osobnych serii do policzenia.", "tips.stat_mtu": "Pakiety odrzucone jako za duże (czarna dziura MTU).", "tips.stat_nat": "Pakiety odrzucone po wygaśnięciu mapowania NAT.", "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.", diff --git a/lang/zh.json b/lang/zh.json index 69273b8..19880c1 100644 --- a/lang/zh.json +++ b/lang/zh.json @@ -303,7 +303,9 @@ "log.loaded_profile": "已加载配置方案", "log.loop": "循环", "log.loss_burst_clamped": "在每串 {burst} 个数据包的情况下，丢包率无法达到 {asked}%，因此本次会话将丢失 {delivered}%。", + "log.loss_burst_clamped_up": "在每串 {burst} 个数据包的情况下，上传丢包率无法达到 {asked}%，因此本次会话的上传将丢失 {delivered}%。", "log.loss_burst_gap": "丢包将以每串约 {burst} 个数据包的方式出现，大约每 {gap} 个数据包出现一串。", + "log.loss_burst_gap_up": "上传丢包将以每串约 {burst} 个数据包的方式出现，大约每 {gap} 个数据包出现一串。", "log.marker_needs_start": "启动会话后才能添加故障标记。", "log.narrow_applied": "当前仅捕获目标流量：驱动只会把指定目标地址的流量交给本工具，因此计数器和连接列表也只覆盖这些流量。", "log.narrow_no_effect": "“仅捕获目标流量”未生效：此目标地址无法转换为驱动过滤器（使用了通配符、re: 正则表达式，或根本未设置目标地址）。将照常捕获全部流量。", @@ -590,7 +592,7 @@ "tips.stat_lan": "在局域网模式下，被丢弃的互联网数据包。", "tips.stat_local": "因“仅互联网”模式而被丢弃的本地网络数据包。", "tips.stat_loss": "因配置的“丢包”效果而被丢弃的数据包。链路中断会在“链路中断”中单独统计。", - "tips.stat_loss_runs": "本次会话产生了多少串连续丢包。如果设置了\"连续丢包\"却显示 0，说明会话太短，还没有出现一串，而不是没有设置。", + "tips.stat_loss_runs": "本次会话产生了多少串连续丢包。如果设置了\"连续丢包\"却显示 0，说明会话太短，还没有出现一串，而不是没有设置。丢包率为 100% 时每个数据包都会丢失，因此没有可单独计数的丢包串。", "tips.stat_mtu": "因超过 MTU（MTU 黑洞）而被丢弃的数据包。", "tips.stat_nat": "NAT 映射过期后被丢弃的数据包。", "tips.stat_overflow": "因队列溢出（严重过载）而被丢弃的数据包。即使视图已收窄到目标，此计数器也始终覆盖所有已捕获流量，因为这些数据包是本工具丢失的。隐藏目标之外的部分会掩盖工具自身造成的损害。", diff --git a/tests/test_burst_loss.py b/tests/test_burst_loss.py index 7dea9d5..ed9cbcb 100644 --- a/tests/test_burst_loss.py +++ b/tests/test_burst_loss.py @@ -20,12 +20,16 @@ deliver half. * **the impossible corner is loud, not quiet.** Some loss/length pairs cannot exist. They are clamped, and ``achievable`` reports what the pair really does. +* **total loss is total.** 100% loses every packet whatever the run length says, + in both directions - and the log and the summary strip, which ask the same + function, stop describing runs that do not exist. * **the chain is re-derived from BOTH of its inputs.** ``p`` depends on the loss as well as the run length, so changing only the loss has to move it. """ +import os import random -from fakes import check +from fakes import ROOT, check from beantester.core import BeanCore, burst_loss_params @@ -114,12 +118,25 @@ def test_a_pair_that_cannot_exist_is_clamped_and_says_so(): f"(achievable={achievable})") -def test_total_loss_does_not_divide_by_zero(): - """``1 - loss`` is the denominator, so 100% is the input that would raise.""" - p, r, achievable = burst_loss_params(1.0, 5.0) - check("total loss keeps the chain in the bad state", p == 1.0, f"(p={p})") - check("and reports total loss", achievable == 1.0, f"(achievable={achievable})") - check("r is still the run length", abs(r - 0.2) < 1e-12, f"(r={r})") +def test_total_loss_is_left_to_the_independent_draw(): + """Total loss has no runs to shape, so it takes the plain draw. + + This test used to assert the opposite - ``(1.0, r, 1.0)``, a chain meant to + "stay bad" - and so pinned the bug the external review found (P1-1): with + ``r > 0`` the chain LEAVES the bad state, and the packet it leaves on goes + through. 100% asked for in runs of 2, 4, 6 and 8 delivered 66.6, 79.9, 85.6 + and 88.9%, while ``achievable`` said 100. ``1 - loss`` is still the + denominator, so this is also still the input that would raise. + """ + for burst in (1.5, 2.0, 5.0, 1000.0): + check(f"total loss in runs of {burst} is the independent draw", + burst_loss_params(1.0, burst) is None, + f"(got {burst_loss_params(1.0, burst)})") + check("and so is anything past total", burst_loss_params(1.5, 5.0) is None, + f"(got {burst_loss_params(1.5, 5.0)})") + check("while just under total is still a chain, clamped out loud", + burst_loss_params(0.9999, 5.0) == (1.0, 0.2, 1.0 / 1.2), + f"(got {burst_loss_params(0.9999, 5.0)})") # --------------------------------------------------------------------------- # @@ -414,3 +431,176 @@ def test_raising_the_upload_loss_does_not_cut_a_download_run_in_flight(): check("while the download keeps its own", abs(_chain(core, False).burst_p - burst_loss_params(0.5, 50.0)[0]) < 1e-12, f"(p={_chain(core, False).burst_p})") + + +# --------------------------------------------------------------------------- # +# Total loss, and the surfaces that ASK the arithmetic about it. The answer is +# per direction and three places read it - the packet path, the apply-time log +# and the summary strip - so each one is driven in both directions: a value that +# depends on its context and has several readers is guarded at the readers, not +# at one of them in one variant. +# --------------------------------------------------------------------------- # +def _upload_core(loss_up, burst): + """A core that loses only on the way UP, through the asymmetry switch.""" + core = BeanCore() + core.set_params(0, 0, 0, 0, 0, 0, 0) + core.set_loss_burst(burst) + core.set_asymmetry(True, loss_up, 0, 0, 0, 0, 0, 0) + core.reset_buckets(0.0) + return core + + +def test_total_loss_loses_every_packet_whatever_the_run_length(): + """Every packet, both ways, and the upload through its own chain. + + Before the fix a run length let 11 to 33% of a "100%" through (external + review, P1-1), and the run counter counted thousands of runs in a session + that has none. + """ + for burst in (2, 6, 8, 1000): + core = _core(loss=100, burst=burst) + dropped, _ = _drops(core, packets=20000, alternate=True) + check(f"100% in runs of {burst} drops every packet in both directions", + dropped == 20000, f"(dropped {dropped} of 20000)") + check(f"100% in runs of {burst} starts no runs, because there are none", + core.loss_bursts == 0, f"({core.loss_bursts})") + dropped, _ = _drops(_upload_core(100, 6), packets=20000) + check("100% UPLOAD loss in runs of 6 drops every upload packet", + dropped == 20000, f"(dropped {dropped} of 20000)") + + +def test_total_loss_makes_one_draw_per_packet_like_the_chain_did(): + """The seed discipline the chain was built around, kept at 100%. + + Step 8 makes ONE draw per packet, independent or in runs, and a stored + ``Reproduce:`` command depends on that count. At 100% nothing after step 8 is + reached, so the generator has to stand exactly where N plain draws leave it. + A shortcut that skipped the draw because "every packet goes anyway" would + shift every draw after it - a scenario stepping back down from 100% would + then replay into a different session. + """ + core = _core(loss=100, burst=6) + rng = random.Random(9) + for i in range(2000): + core.decide(1200, bool(i % 2), 5000, i * 0.001, rng, + remote_ip="1.2.3.4", remote_port=443, is_tcp=True) + expected = random.Random(9) + for _ in range(2000): + expected.random() + check("2000 packets at 100% in runs of 6 made exactly 2000 draws", + rng.getstate() == expected.getstate(), "(the draw count moved)") + + +def test_the_shipped_lte_to_3g_outage_loses_everything(): + """The case the review found in a file this project ships. + + ``mobile-lte-to-3g.json`` cuts the link at 60 s with ``"loss": 100`` and + inherits a run length of 8 from the step before, so its full outage let about + one packet in nine through - enough for a connection to live through it. + Driven the way a session drives it: scenario step, ``apply_settings``, engine, + core. + """ + from beantester.engine import BeanEngine + from beantester.scenario import load_scenario_file + from beantester.settings import DEFAULT_SETTINGS, apply_settings + + scenario = load_scenario_file(os.path.join(ROOT, "scenarios", + "mobile-lte-to-3g.json")) + settings = scenario.settings_at(60.05, DEFAULT_SETTINGS) + check("the step is still total loss with an inherited run length", + settings["loss"] == 100 and settings["loss_burst"] > 1, + f"(loss={settings['loss']}, run={settings['loss_burst']})") + engine = BeanEngine() + apply_settings(engine, settings) + engine.core.reset_buckets(0.0) + dropped, _ = _drops(engine.core, packets=20000, alternate=True) + check("the outage drops every packet", dropped == 20000, + f"(dropped {dropped} of 20000)") + + +def _burst_lines(monkeypatch, **fields): + """What an apply says about runs of loss, as ``(key, values)`` pairs. + + ``T`` is swapped for a recorder, so the assertions name the MESSAGE and its + numbers rather than one translation of them - the language the suite happens + to run in is not the question here. + """ + import beantester.settings as settings_mod + from beantester.engine import BeanEngine + monkeypatch.setattr(settings_mod, "T", lambda key, **values: (key, values)) + said = [] + settings_mod.apply_settings(BeanEngine(), + dict(settings_mod.DEFAULT_SETTINGS, **fields), + log=said.append) + return [line for line in said + if isinstance(line, tuple) and line[0].startswith("log.loss_burst")] + + +def test_total_loss_says_nothing_about_runs(monkeypatch): + """At 100% the log used to promise "runs of about 6, one run every 6 packets". + + A run length at total loss is as inert as one at no loss, and gets the same + silence, in either direction. + """ + down = _burst_lines(monkeypatch, loss=100, loss_burst=6) + check("download loss of 100% in runs of 6 says nothing about runs", + down == [], f"({down})") + up = _burst_lines(monkeypatch, asym=True, loss=0, loss_up=100, loss_burst=6) + check("and neither does an upload loss of 100%", up == [], f"({up})") + + +def test_an_upload_the_runs_cannot_carry_is_said_out_loud(monkeypatch): + """The upload walks its own chain from its own loss, so it clamps on its own. + + Until the external review (P3-4) only the download was asked, and 95% upload + loss in runs of 5 delivered 83% in silence. + """ + lines = _burst_lines(monkeypatch, asym=True, loss=5, loss_up=95, loss_burst=5) + check("the download says its gap, then the upload its clamp and its gap", + [key for key, _ in lines] == ["log.loss_burst_gap", + "log.loss_burst_clamped_up", + "log.loss_burst_gap_up"], f"({lines})") + said = dict(lines) + clamp = said.get("log.loss_burst_clamped_up", {}) + check("the clamp names what was asked and what the upload will really lose", + clamp.get("asked") == "95" and clamp.get("delivered") == "83.33", + f"({clamp})") + check("the upload's gap follows from what it really loses", + said.get("log.loss_burst_gap_up", {}).get("gap") == "6", f"({said})") + check("while the download keeps its own gap", + said.get("log.loss_burst_gap", {}).get("gap") == "100", f"({said})") + + +def test_upload_values_the_session_does_not_read_are_not_said(monkeypatch): + """With asymmetry off the upload fields are not read at all. + + A value left there from an earlier try is the ordinary path, not a corner + case, and a line about it would describe a link this session never produces. + The download, which then means both directions, is still said. + """ + lines = _burst_lines(monkeypatch, asym=False, loss=90, loss_up=95, loss_burst=5) + check("only the download lines, with the upload left over and switched off", + [key for key, _ in lines] == ["log.loss_burst_clamped", "log.loss_burst_gap"], + f"({lines})") + + +def test_the_summary_strip_names_the_runs_each_direction_really_gets(): + """The strip asks the same function, per direction, so it cannot claim runs + the engine is not producing - and does not hide the ones it is.""" + from beantester.summary import settings_summary + total = settings_summary(dict(loss=100, loss_burst=6), "en") + check("100% loss claims no runs", + "100% loss" in total and "in runs of" not in total, f"({total!r})") + down = settings_summary(dict(loss=5, loss_burst=20), "en") + check("a download loss in runs says so right after it", + "5% loss, in runs of 20 packets" in down, f"({down!r})") + up = settings_summary(dict(asym=True, loss_up=5, loss_burst=20, corrupt_up=1), "en") + check("an upload loss in runs says so too, in the same place", + "upload: 5% loss, in runs of 20 packets, 1% corruption" in up, f"({up!r})") + up_total = settings_summary(dict(asym=True, loss_up=100, loss_burst=6), "en") + check("100% upload loss claims no runs either", + "upload: 100% loss" in up_total and "in runs of" not in up_total, + f"({up_total!r})") + off = settings_summary(dict(asym=False, loss_up=5, loss_burst=20), "en") + check("with asymmetry off a leftover upload loss is not described", + "upload" not in off, f"({off!r})") diff --git a/tests/test_mutation_registry.py b/tests/test_mutation_registry.py index 085f26c..404ddf8 100644 --- a/tests/test_mutation_registry.py +++ b/tests/test_mutation_registry.py @@ -2386,6 +2386,67 @@ "new": " bad = True", "test": "test_the_run_counter_answers_did_this_fire_at_all", }, + { + # The shape the external review found (P1-1): a chain told to "stay bad" + # at 100% still leaves the bad state at r and lets that packet through. + "label": "burst loss: total loss walks the chain and lets packets through", + "file": "beantester/core.py", + "old": " # way. This branch is also what keeps the division below from raising.\n" + " return None", + "new": " # way. This branch is also what keeps the division below from raising.\n" + " return (1.0, r, 1.0)", + "test": "test_total_loss_loses_every_packet_whatever_the_run_length", + }, + { + # The upload walks its own chain from its own loss, so it clamps on its + # own - and until P3-4 the apply log never asked about it. + "label": "burst loss: the upload's clamp and gap go unsaid", + "file": "beantester/settings.py", + "old": " if g(\"asym\"):\n" + " _say_burst_loss_for(g(\"loss_up\"), g(\"loss_burst\"), _BURST_LINES_UP, log)", + "new": " pass", + "test": "test_an_upload_the_runs_cannot_carry_is_said_out_loud", + }, + { + # The other half of the same helper: with the switch off the upload values + # are not read, so a line about them describes a link nobody is producing. + "label": "burst loss: leftover upload values are said with asymmetry off", + "file": "beantester/settings.py", + "old": " if g(\"asym\"):\n" + " _say_burst_loss_for(g(\"loss_up\")", + "new": " if True:\n" + " _say_burst_loss_for(g(\"loss_up\")", + "test": "test_upload_values_the_session_does_not_read_are_not_said", + }, + { + # The upload half of the strip said its loss without its run length, so an + # upload losing in runs read exactly like one losing evenly (P3-4). + "label": "summary: the upload loss is described without its run length", + "file": "beantester/summary.py", + "old": " + _loss_parts(g, tr, num, \"loss_up\")", + "new": " + _plain_parts(g, tr, num, ((\"loss_up\", \"summary.loss\"),))", + "test": "test_the_summary_strip_names_the_runs_each_direction_really_gets", + }, + { + # A threshold instead of asking the function that DECIDES: the strip then + # claims runs the engine is not producing - at 100% loss, for one. + "label": "summary: the upload's run length is compared instead of asked", + "file": "beantester/summary.py", + "old": " if burst_loss_params(to_number(g(key)) / 100.0,\n" + " to_number(g(\"loss_burst\"))) is not None:", + "new": " if to_number(g(\"loss_burst\")) > 1.0:", + "test": "test_the_summary_strip_names_the_runs_each_direction_really_gets", + }, + { + # The same shortcut in the download half, which asks inline (it says there + # why): at 100% loss the strip would promise runs the engine never makes. + "label": "summary: the download's run length is compared instead of asked", + "file": "beantester/summary.py", + "old": " if burst_loss_params(to_number(g(\"loss\")) / 100.0,\n" + " to_number(g(\"loss_burst\"))) is not None:", + "new": " if to_number(g(\"loss_burst\")) > 1.0:", + "test": "test_the_summary_strip_names_the_runs_each_direction_really_gets", + }, { # The plainest way to break convention 36, and the one a session in a # hurry would reach for: an update check, a crash reporter, a "quick