test(dns): compare every question type, not just A - #245
Merged
Conversation
This file exists to make drift between src/dns.mjs and the published
@moshcoder/moshpit-dns loud instead of silent. It asked an A question,
compared the answer, and passed — through a release where one copy learned
to answer AAAA and the other did not, and a second where one learned CNAME,
MX and TXT and the other did not. A resolver comparison that only ever asks
one question type is a comparison of one question type.
So it now asks every type either copy claims to serve:
- RECORD_TYPES is compared directly, so a type one copy grows and the
other does not fails before any wire byte is looked at.
- AAAA answers are compared byte for byte, which is the gap that let
AAAA ship on one side alone.
- The NODATA/NXDOMAIN split is compared, because NXDOMAIN is cached and
applied to every type — one copy denying a name the other merely has
nothing to say about is not a cosmetic difference.
- TXT rdata is compared past one string and across a multi-byte
character on the 255-byte boundary: the two ways a split goes wrong,
both producing a reply of the right shape that no client can read.
- Oversized answers are compared, where one copy dropping every answer
and the other keeping what fits both read as "truncated" to anything
checking the flag alone.
- `records=1` is asserted to stay opt-in in both. Every DNS query on the
machine goes through resolveName, so a copy that started asking for
records unconditionally would put a second registry query on every
page load, and nothing here would have noticed.
- The shared surface is named explicitly. The vendored copy is a
superset — it also forwards, rate limits and writes catch-all routing
— so the part that must exist on both sides is listed rather than
inferred, and dropping one from the package fails.
Verified against the version this used to pass: 7 subtests fail on
@moshcoder/moshpit-dns@0.2.2 and all 19 pass on 0.3.0, which is the
release that closed the gap. devDependency moved to ^0.3.0 to match —
^0.2.2 does not admit 0.3.0, so the test would otherwise have gone on
comparing against the copy it was written to catch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
CI installs with --frozen-lockfile, which refuses a lockfile whose specifier disagrees with package.json. Bumping the devDependency without regenerating the lock failed the run before a single test ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ralyodio
marked this pull request as ready for review
August 3, 2026 08:30
ralyodio
added a commit
that referenced
this pull request
Aug 3, 2026
apps/pwa's drift test has failed 3 subtests for a while, and the reason was never the vendored copy: `@moshcoder/moshpit-name` fixed the label rule — no dashes, because a dash is the cheapest way to mint a near-miss of an ending someone else holds — and never published it. npm served the old permitting regex, the test compared against that, and reported the fixed copy as the one out of step. Published now as 0.4.0. The specifier has to move with it: `^0.2.0` cannot resolve `0.4.0` on a 0.x range, so leaving it would go on comparing against the copy the release exists to replace. That caret is what hid this — the same trap that hid the DNS bridge divergence in #245. apps/pwa: 456 pass, 0 fail, 0 skipped. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The last item from the DNS Records work (#241, #242, moshcoder/moshpit-dns#10), now unblocked by
@moshcoder/moshpit-dns@0.3.0being on npm.The problem
test/dns-drift.test.mjsexists to make drift betweensrc/dns.mjsand the published@moshcoder/moshpit-dnsloud instead of silent. Its own header says "behaviour rather than bytes... requires the same answers."It asked an A question, compared the answer, and passed — through a release where one copy learned to answer AAAA and the other didn't, and a second where one learned CNAME/MX/TXT and the other didn't. A resolver comparison that only ever asks one question type is a comparison of one question type.
This is what let me spend a while believing the two copies were in sync when they weren't.
What it checks now
Every type either copy claims to serve:
RECORD_TYPEScompared directly — a type one copy grows and the other doesn't fails before any wire byte is examined.records=1stays opt-in in both — every DNS query on the machine goes throughresolveName, so a copy that started asking for records unconditionally would put a second registry query on every page load, and nothing here would have noticed.Verified against the version it used to pass
This is the part that matters: 7 subtests fail on
@moshcoder/moshpit-dns@0.2.2, which the old test passed cleanly. All 19 pass on0.3.0.The
devDependencymoves to^0.3.0—^0.2.2does not admit0.3.0on a 0.x version, so without this the test would have gone on comparing against the very copy it was written to catch.Tests
test/dns*.test.mjs+test/doh*.test.mjs: 182/182 pass.🤖 Generated with Claude Code