getlantern / getlantern/lantern-box

Lanturn outbound v0.1 — test coverage gaps deferred from PR #257

Open
#264 0 comments 0 reactions 0 assignees View on GitHub

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_UnreachableCoturn timeout 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 (MediaProfile cast at outbound.go:131) has no validation. If upstream.MediaProfile ever 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.