You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Follow-ups to #268, #260 and #212: pktEsc listener leak, test gaps and comment fixes #282
I1 — pktEsc leaks a listener. It is added in the packets page init() (public/packets.js ~2363) and has the same leak class as nodesEsc/nodesPanelEsc: a fresh named closure on document per init, never removed. Fix it the same way (at most one listener) and add a test.
N2 — an untested guard. The "only while a detail panel is open" guard in _nodesPanelEsc has no test, so mutant ND survives. Add one.
From #268 (#258):
4. A wrong test header. The header comment of test-issue-258-column-widths-e2e.js describes dragging the Path handle left, but the test drags the Time handle right. Fix the comment.
5. A wrong PR claim. The PR text claims that analytics tables "never create an observer". Tables with fewer than 5 usable rows do. Correct it in the docs or code comments where the claim is repeated.
From #212:
6. N1 — an untested transport-route frame.substr(t.raw_hex, 1, 12) in cmd/server/db.go (~3583/3591) is exactly the minimum a transport route needs, but no test sends a transport-route frame through GetChannelMessages. Add one.
7. N2 — two offset sources. The Path Length row in the hex breakdown (public/packets.js ~3875–3897) mixes two offset sources: off from pkt.route_type, and senderPathHashSize(buf). Make them consistent, or add a test that pins that they agree.
8. N3 — exports used only by tests.renderObservedPathHashBadge and OBSERVED_PATH_HASH_TOOLTIP in public/channels.js are only reachable from the test export. Remove them, or document why they are kept.
Nits collected from the reviews of #268, #260 (round 2) and #212 (round 2). Details are in the review comments on each PR.
From #260 (#259):
pktEscleaks a listener. It is added in the packets pageinit()(public/packets.js~2363) and has the same leak class asnodesEsc/nodesPanelEsc: a fresh named closure ondocumentper init, never removed. Fix it the same way (at most one listener) and add a test.groupIsExpandedInView()(public/packets.js~2420) still names the bug: /#/packets/<hash> full-page — clicking a different observation doesn't update hex payload or path details Kpa-clawbot/CoreScope#866 deep link as#/packets/<hash>/<obs>. The correct form is#/packets/<hash>?obs=<id>._nodesPanelEschas no test, so mutant ND survives. Add one.From #268 (#258):
4. A wrong test header. The header comment of
test-issue-258-column-widths-e2e.jsdescribes dragging the Path handle left, but the test drags the Time handle right. Fix the comment.5. A wrong PR claim. The PR text claims that analytics tables "never create an observer". Tables with fewer than 5 usable rows do. Correct it in the docs or code comments where the claim is repeated.
From #212:
6. N1 — an untested transport-route frame.
substr(t.raw_hex, 1, 12)incmd/server/db.go(~3583/3591) is exactly the minimum a transport route needs, but no test sends a transport-route frame throughGetChannelMessages. Add one.7. N2 — two offset sources. The Path Length row in the hex breakdown (
public/packets.js~3875–3897) mixes two offset sources:offfrompkt.route_type, andsenderPathHashSize(buf). Make them consistent, or add a test that pins that they agree.8. N3 — exports used only by tests.
renderObservedPathHashBadgeandOBSERVED_PATH_HASH_TOOLTIPinpublic/channels.jsare only reachable from the test export. Remove them, or document why they are kept.