Skip to content

test(x): mock StrategyFinder dialers in smart integration tests#627

Merged
fortuna merged 3 commits into
mainfrom
codex/tls-expired-cert-local
Jun 18, 2026
Merged

test(x): mock StrategyFinder dialers in smart integration tests#627
fortuna merged 3 commits into
mainfrom
codex/tls-expired-cert-local

Conversation

@fortuna

@fortuna fortuna commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Simplify smart integration tests by using mock dialers and an error fallback parser. This removes the need for the nettest build tag and external network connections.

@fortuna fortuna force-pushed the codex/tls-expired-cert-local branch from 95ea53d to eac3aee Compare June 16, 2026 22:20
@fortuna fortuna requested a review from ohnorobo June 16, 2026 22:20
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR converts smart strategy finder integration tests and two TLS stream dialer tests from live-network to fully hermetic by introducing mock dialers and local httptest servers. The test logic is sound — error chains propagate correctly through the dns.nestedError wrapping, and the mock scenarios accurately replicate the failure conditions being tested.

Confidence Score: 4/5

Safe to merge — all changes are in test files and the mock logic correctly replicates the failure scenarios the tests were designed to exercise.

The conversion from live-network tests to hermetic mocks is logically sound: error chains are traced correctly, the captive-portal server closes connections immediately reproducing the right TLS failure, and the untrusted-cert server uses httptest self-signed cert as intended. The only issues are a misordered import block in stream_dialer_test.go and a semantically misleading error string in mockStreamDialer's fallback branch — neither affects correctness or CI.

The import ordering in transport/tls/stream_dialer_test.go is the only thing worth a quick second look.

Important Files Changed

Filename Overview
x/smart/integrationtest/integration_test.go Removes nettest build tag and replaces live network calls with mock dialers; logic is correct but the generic error message used by the mock for non-DNS connections is slightly misleading.
transport/tls/stream_dialer_test.go Converts two live-network TLS tests to hermetic local-server variants using generated certs; imports are not in standard alphabetical order within the stdlib group.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Test as Test
    participant SF as StrategyFinder
    participant MD as mockStreamDialer
    participant EPD as errorPacketDialer
    participant CP as CaptivePortalServer (TCP)
    participant TS as httptest.TLSServer (untrusted)

    Test->>SF: NewDialer(ctx, testDomains, configBytes)

    Note over SF: DNS phase
    SF->>EPD: DialPacket(china.cn:53 / ns1.tic.ir:53 / tmcell.tm:53)
    EPD-->>SF: error(dial DNS resolver failed)

    SF->>MD: DialStream(captive-portal.badssl.com:443)
    MD->>CP: DialStream(127.0.0.1:PORT)
    CP-->>MD: accept + close
    MD-->>SF: conn then EOF on TLS wrap

    SF->>MD: DialStream(mitm-software.badssl.com:443)
    MD->>TS: DialStream(127.0.0.1:PORT)
    TS-->>MD: TCP conn
    MD-->>SF: conn (TLS fails - untrusted cert)

    Note over SF: TLS strategy phase
    SF->>MD: DialStream(www.example.com:443)
    MD-->>SF: error (all strategies fail)

    Note over SF: Fallback phase
    SF->>MD: DialStream(1.2.3.4:9999) SS proxy
    MD-->>SF: error
    SF->>SF: error parser returns failed to start dialer
    SF->>MD: DialStream(192.168.1.10:1080) socks5
    MD-->>SF: error

    SF-->>Test: error(could not find a working fallback: all tests failed)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Test as Test
    participant SF as StrategyFinder
    participant MD as mockStreamDialer
    participant EPD as errorPacketDialer
    participant CP as CaptivePortalServer (TCP)
    participant TS as httptest.TLSServer (untrusted)

    Test->>SF: NewDialer(ctx, testDomains, configBytes)

    Note over SF: DNS phase
    SF->>EPD: DialPacket(china.cn:53 / ns1.tic.ir:53 / tmcell.tm:53)
    EPD-->>SF: error(dial DNS resolver failed)

    SF->>MD: DialStream(captive-portal.badssl.com:443)
    MD->>CP: DialStream(127.0.0.1:PORT)
    CP-->>MD: accept + close
    MD-->>SF: conn then EOF on TLS wrap

    SF->>MD: DialStream(mitm-software.badssl.com:443)
    MD->>TS: DialStream(127.0.0.1:PORT)
    TS-->>MD: TCP conn
    MD-->>SF: conn (TLS fails - untrusted cert)

    Note over SF: TLS strategy phase
    SF->>MD: DialStream(www.example.com:443)
    MD-->>SF: error (all strategies fail)

    Note over SF: Fallback phase
    SF->>MD: DialStream(1.2.3.4:9999) SS proxy
    MD-->>SF: error
    SF->>SF: error parser returns failed to start dialer
    SF->>MD: DialStream(192.168.1.10:1080) socks5
    MD-->>SF: error

    SF-->>Test: error(could not find a working fallback: all tests failed)
Loading

Comments Outside Diff (1)

  1. transport/tls/stream_dialer_test.go, line 25-28 (link)

    P2 io and log are inserted after math/big, but alphabetically (i < l < m) they should come before it. goimports would reorder them, which can cause noisy diffs or CI failures if an import-lint check is enforced.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "test: mock StrategyFinder dialers in sma..." | Re-trigger Greptile

Comment thread x/smart/integrationtest/integration_test.go
@fortuna fortuna changed the title test: mock StrategyFinder dialers in smart integration tests test(x): mock StrategyFinder dialers in smart integration tests Jun 16, 2026
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@fortuna fortuna enabled auto-merge (squash) June 17, 2026 16:35

@ohnorobo ohnorobo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the flakiness fix!

@fortuna fortuna merged commit aa3171c into main Jun 18, 2026
7 checks passed
@fortuna fortuna deleted the codex/tls-expired-cert-local branch June 18, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants