Skip to content

refactor: inject http.Client into CheckExternalLinks to enable httptest.NewTestServer #410

Description

@shinagawa-web

Overview

CheckExternalLinks constructs its own *http.Client internally, making it impossible to inject a test client from outside.
Go 1.27 introduced httptest.NewTestServer, whose Client() routes requests to the in-process server transparently — so tests can use real production URLs instead of replacing them with the test server address.
To take advantage of this, the function must accept a client from the caller.

Problem

// internal/rule/external_link.go
client := &http.Client{
    Timeout: time.Duration(timeoutSeconds) * time.Second,
}

Because client is created inside CheckExternalLinks, tests are forced to rewrite every URL in the test markdown to point to the temporary localhost server:

markdown := fmt.Sprintf(`[ok](%s/ok)`, ts.URL)  // real URL is lost

This means tests cannot verify that production code actually requests the correct host or path.

Proposed change

Add a *http.Client parameter to CheckExternalLinks.
When nil is passed, the function falls back to its current behaviour (constructs a default client using timeoutSeconds).

func CheckExternalLinks(
    path string, ctx *preprocess.Context, offset int,
    skipPatterns []*regexp.Regexp,
    timeoutSeconds, retryDelayMs, maxConcurrency, maxRetries int,
    allowedStatuses []int, urlCache *sync.Map,
    perHostConcurrency, perHostIntervalMs int,
    client *http.Client,   // nil → build default
) ([]LintError, int)

Effect on tests (Go 1.27+)

ts := httptest.NewTestServer(handler)
// ts.Client() routes any host to the in-process server
results, _ := rule.CheckExternalLinks(
    "mock.md", ctx, 0,
    nil, 10, 10, rule.DefaultMaxConcurrency, rule.DefaultMaxRetries,
    nil, &sync.Map{}, 0, 0,
    ts.Client(),   // injected
)

Markdown in tests can now contain literal production-like URLs (https://example.com/ok) while still being served by the test server — letting tests assert on method, host, and path correctness.

Effect on the linter (production)

// internal/linter/linter.go — pass nil; behaviour unchanged
rule.CheckExternalLinks(..., nil)

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions