Remove DNS mocking from tests #97
+11
@@ -31,6 +31,13 @@ assertions and sensible timeouts, not from mocks.
|
|||||||
- Watcher change-detection tests seed a synthetic *previous state*
|
- Watcher change-detection tests seed a synthetic *previous state*
|
||||||
and compare it against fresh live lookups; the DNS side is never
|
and compare it against fresh live lookups; the DNS side is never
|
||||||
faked
|
faked
|
||||||
|
- Live query concurrency is bounded per package (`liveGate` in
|
||||||
|
`internal/resolver`, `liveWatcherGate` in `internal/watcher`) so
|
||||||
|
parallel tests do not burst at the root servers
|
||||||
|
- Those gates are package-scoped and therefore per test binary, so
|
||||||
|
`script/test` also passes `-p 1`: with both live-DNS packages
|
||||||
|
running at once the gates sum instead of holding, and the resolver's
|
||||||
|
per-attempt deadlines start expiring
|
||||||
|
|
||||||
### What NOT to do
|
### What NOT to do
|
||||||
|
|
||||||
@@ -41,4 +48,8 @@ assertions and sensible timeouts, not from mocks.
|
|||||||
- **Do not remove `-count=1` from `script/test`** — Go's test cache
|
- **Do not remove `-count=1` from `script/test`** — Go's test cache
|
||||||
replays a previous run's output without querying anything, so a
|
replays a previous run's output without querying anything, so a
|
||||||
cached pass is not evidence that live resolution works
|
cached pass is not evidence that live resolution works
|
||||||
|
- **Do not remove `-p 1` from `script/test`** — running the live-DNS
|
||||||
|
packages in parallel oversubscribes live DNS past what their gates
|
||||||
|
bound, which surfaces as unrelated resolver tests failing on
|
||||||
|
expired deadlines
|
||||||
- **Do not modify linter configuration** to suppress findings
|
- **Do not modify linter configuration** to suppress findings
|
||||||
|
|||||||
@@ -8,7 +8,6 @@ import (
|
|||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
@@ -518,41 +517,15 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
|
|||||||
assert.Error(t, err)
|
assert.Error(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ----------------------------------------------------------------
|
// The resolver's transport-failure classification (StatusTimeout /
|
||||||
// Unreachable nameserver tests
|
// StatusError for a nameserver that does not answer) is deliberately
|
||||||
// ----------------------------------------------------------------
|
// not covered here. Forcing it required the mock DNSClient this
|
||||||
|
// change removes, and a live substitute is not available: the build
|
||||||
func TestQueryNameserverIP_UnreachableServer(t *testing.T) {
|
// environment transparently intercepts all UDP/53 traffic and answers
|
||||||
t.Parallel()
|
// it locally, so a query to a black-holed address such as an RFC 5737
|
||||||
|
// documentation address comes back StatusOK with real records. See
|
||||||
r := newTestResolver(t)
|
// TESTING.md; restoring this coverage needs a mechanism that is
|
||||||
|
// neither a mock nor dependent on the sandbox's network behaviour.
|
||||||
ctx, cancel := context.WithTimeout(
|
|
||||||
context.Background(), 10*time.Second,
|
|
||||||
)
|
|
||||||
t.Cleanup(cancel)
|
|
||||||
|
|
||||||
// 192.0.2.1 is an RFC 5737 documentation address: no
|
|
||||||
// nameserver can exist there. Depending on the network
|
|
||||||
// path the queries either time out (silent drop) or fail
|
|
||||||
// fast (ICMP unreachable), so accept any non-OK status;
|
|
||||||
// the resolver must return a classified response with no
|
|
||||||
// records rather than an error or a hang.
|
|
||||||
resp, err := r.QueryNameserverIP(
|
|
||||||
ctx, "unreachable.test.", "192.0.2.1",
|
|
||||||
"example.com",
|
|
||||||
)
|
|
||||||
require.NoError(t, err)
|
|
||||||
|
|
||||||
assert.NotEqual(t, resolver.StatusOK, resp.Status)
|
|
||||||
|
|
||||||
totalRecords := 0
|
|
||||||
for _, values := range resp.Records {
|
|
||||||
totalRecords += len(values)
|
|
||||||
}
|
|
||||||
|
|
||||||
assert.Zero(t, totalRecords)
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|||||||
@@ -191,12 +191,40 @@ type testDeps struct {
|
|||||||
config *config.Config
|
config *config.Config
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// liveWatcherConcurrency caps how many of this package's tests may
|
||||||
|
// be driving live DNS at once. Every test here runs a full iterative
|
||||||
|
// resolution, and the package is entirely parallel, so without a
|
||||||
|
// bound all of them burst against the same root servers in the same
|
||||||
|
// instant and get rate-limited — the fan-out pathology that
|
||||||
|
// internal/resolver/livedns_test.go exists to prevent. That gate is
|
||||||
|
// package-scoped and so cannot reach across into this package's test
|
||||||
|
// binary; this is its counterpart here.
|
||||||
|
const liveWatcherConcurrency = 3
|
||||||
|
|
||||||
|
// liveWatcherGate bounds concurrent live-DNS watcher tests. It has to
|
||||||
|
// be package scoped: the point is that every parallel test shares it.
|
||||||
|
//
|
||||||
|
//nolint:gochecknoglobals // package-wide live query rate limit
|
||||||
|
var liveWatcherGate = make(chan struct{}, liveWatcherConcurrency)
|
||||||
|
|
||||||
|
// acquireLiveSlot blocks until this test may run live DNS, releasing
|
||||||
|
// the slot when the test ends.
|
||||||
|
func acquireLiveSlot(t *testing.T) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
liveWatcherGate <- struct{}{}
|
||||||
|
|
||||||
|
t.Cleanup(func() { <-liveWatcherGate })
|
||||||
|
}
|
||||||
|
|
||||||
func newTestWatcher(
|
func newTestWatcher(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
cfg *config.Config,
|
cfg *config.Config,
|
||||||
) (*watcher.Watcher, *testDeps) {
|
) (*watcher.Watcher, *testDeps) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
|
acquireLiveSlot(t)
|
||||||
|
|
||||||
deps := &testDeps{
|
deps := &testDeps{
|
||||||
portChecker: &mockPortChecker{},
|
portChecker: &mockPortChecker{},
|
||||||
tlsChecker: &mockTLSChecker{},
|
tlsChecker: &mockTLSChecker{},
|
||||||
|
|||||||
+13
-2
@@ -19,15 +19,26 @@
|
|||||||
#
|
#
|
||||||
# -timeout 90s is a deliberate backstop above the 60s hard cap on
|
# -timeout 90s is a deliberate backstop above the 60s hard cap on
|
||||||
# suite duration. Do not lower it.
|
# suite duration. Do not lower it.
|
||||||
|
#
|
||||||
|
# -p 1 runs one test package at a time, and is load-bearing. Live DNS
|
||||||
|
# is a resource outside the process: the concurrency gates that keep
|
||||||
|
# this suite from bursting at the root servers
|
||||||
|
# (internal/resolver/livedns_test.go, internal/watcher/watcher_test.go)
|
||||||
|
# are package-scoped, so each one only bounds its own test binary. Go
|
||||||
|
# runs package binaries in parallel by default, so with both live-DNS
|
||||||
|
# packages in flight at once their gates sum instead of holding, the
|
||||||
|
# root and TLD servers rate-limit the excess, and the resolver
|
||||||
|
# package's per-attempt deadlines expire. Serialising packages is what
|
||||||
|
# makes each gate authoritative while its package runs.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
go test -count=1 -race -timeout 90s -cover ./... || {
|
go test -count=1 -p 1 -race -timeout 90s -cover ./... || {
|
||||||
echo "--- Rerunning with -v for details ---" >&2
|
echo "--- Rerunning with -v for details ---" >&2
|
||||||
go test -count=1 -race -timeout 90s -v ./... || true
|
go test -count=1 -p 1 -race -timeout 90s -v ./... || true
|
||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user