getlantern / getlantern/lantern-box
Lanturn outbound v0.1 — test coverage gaps deferred from PR #257
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 14
- Forks
- 5
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 13
Description
Tracking issue for test coverage gaps in the v0.1 lanturn outbound (landed in #257). The PR's 7 tests cover the negative contracts the original review feedback called out (config validation, UDP rejection, error wrapping, Network() advertisement, supportedProtocols registration), but leave the positive paths from commit 772ae48 ("wire real DialContext via M.SocksaddrSerializer") with no direct coverage. None of these block #257 from merging — they're deferred-from-v0.1 with eyes open. This issue exists so they don't get lost.
Gaps to close
1. test/e2e/lanturn_test.go is missing
The 772ae48 commit message says:
End-to-end success path tested by
test/e2e/lanturn_test.go(Phase-5 follow-up, in-process).
That file doesn't exist in the PR. The end-to-end happy path is genuinely untested today. Probably the highest-value test in the project — would catch the whole class of "DialContext signature changed but destination forwarding silently broke."
Shape: in-process coturn (e.g. github.com/pion/turn/v3 server in a goroutine) + a tiny echo TCP server on the egress side. Dial through the outbound, write/read, assert round-trip. Needs no real network and runs in <2s.
2. Positive test for destination forwarding
The load-bearing change in 772ae48 is "destination now reaches the egress via M.SocksaddrSerializer." There's no unit test that asserts this. If upstream.Dial(ctx, cfg, destination) were accidentally changed to upstream.Dial(ctx, cfg) (drop destination) — or if the destination got mangled in transit — only the missing e2e would catch it.
Shape: introduce a test seam for upstream.Dial (function-typed var that the test overrides) and assert the destination arg matches the M.Socksaddr passed to DialContext. ~30 lines.
3. Credential callback shape test
NewOutbound constructs a Credential callback at protocol/lanturn/outbound.go:135-145 that returns {Username: "", Password: opts.LanturnAuthSecret}. If someone swaps the field order or empties the password, no test catches it.
Shape: drive cfg.Credential(upstream.CoturnEndpoint{...}) directly in the test, assert the returned Credential matches what we passed in LanturnAuthSecret. ~10 lines.
4. ListenPacket rejection test
ListenPacket returns "lanturn: ListenPacket not supported" (outbound.go:195). Untested. Pinning the contract is one-liner work.
Suggested PR splits
- Quick PR: tests #3 + #4 above. Both are pure unit tests with no infra dependency. Maybe 50 lines total.
- Bigger PR: tests #1 + #2 together. They share the same in-process coturn setup; doing them in one PR keeps the test-helper code in one place.
Other minor notes
- The
TestDialContext_UnreachableCoturntimeout is 6s. Fine on dev machines; could flake on slow CI runners. Worth tightening to 2s or measuring the actual failure latency once and pinning that. Profile(MediaProfilecast at outbound.go:131) has no validation. Ifupstream.MediaProfileever needs to reject unknown profile strings, we'd want a test for it. Acceptable v0.1.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with protocol/lanturn/outbound.go, especially NewOutbound at lines 135-145 and ListenPacket at line 195, then review the existing lanturn tests and run TestDialContext_UnreachableCoturn. Done means the Credential and ListenPacket contracts are covered, destination forwarding is asserted, and the in-process happy path in test/e2e/lanturn_test.go exercises a round trip without real network access.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, testing-qa
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100