fix(test): unblock Windows type-checking and fix the arm64 sockaddr assertions - #89
Merged
Merged
Conversation
added 2 commits
July 29, 2026 22:52
netutils_test.go imported golang.org/x/sys/unix with no build constraint, purely to name unix.AF_INET/unix.AF_INET6. That package has essentially no files selected for GOOS=windows, so those identifiers are undefined there and the entire srtgo test package failed to type-check on Windows -- all 26 tests, not just the two that used it. The library itself was never affected; netutils_unix.go is the only non-test importer and it is //go:build !windows. Use the package-internal afINET4/afINET6 constants, which netutils.go already uses for exactly these values and which are defined for both build tags. This makes the test compile and run on Windows rather than hiding it behind a //go:build !windows tag, and it asserts the same constant the code under test writes (windows.AF_INET6 is 23, not the 10 x/sys/unix would have claimed). Add portability_test.go, which walks every *_test.go that go/build selects for GOOS=windows and fails if it imports a package with no Windows implementation, and correct the now-false CI comment that documented this as a live condition.
CreateAddrInet produces byte-identical sockaddrs on linux/amd64 and linux/arm64; nothing in netutils.go is wrong on arm64 and no user is affected. The two tests failed there because they hardcoded expected sa_data bytes as signed decimals (-102, -72, -123, -93) and compared through int(). sa_data is C char, whose signedness is ABI-defined: cgo maps it to int8 where plain char is signed (x86-64 System V, Apple arm64) and to uint8 where it is unsigned (AArch64 Linux / AAPCS64). On the latter int(uint8(0x9a)) is 154 and can never equal -102. Express the expectations as raw bytes and compare through byte(), which is value-preserving for the same bit pattern under either mapping, and report index/got/want so the next platform surprise is diagnosable. The expected bit patterns are unchanged, and index 1 is now genuinely asserted on linux/arm64 where it previously could only ever mismatch. Note CI covers ubuntu-latest (amd64) and macos-latest (arm64, signed char); Linux+AArch64 is the untested combination, so this can regress silently again without an arm64 runner.
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.
Two test-only fixes. No library code changes, so no runtime risk.
1.
netutils_test.goblockedgo teston WindowsIt imported
golang.org/x/sys/unixwith no build tag and usedunix.AF_INET/unix.AF_INET6. UnderGOOS=windowsthat package has no such constants, so the whole test package failed to type-check.go buildof the library was unaffected — consumers were fine — but the tests could never run there.The package already defines
afINET4/afINET6for both build tags, so the test now uses those and drops the import. That makes the test run on Windows rather than hiding it behind//go:build !windows, and it is also more correct on its own terms: on Windowsunix.AF_INET6would be 10, while the valuesockAddrFromIp6actually writes iswindows.AF_INET6== 23.portability_test.goparses every*_test.gounder aGOOS=windowsbuild context and fails if any imports a package that cannot build there, so this cannot silently regress.It also updates the CI comment that named this as the reason for having no Windows job — that reason is now gone. What remains is the toolchain: libsrt on a Windows runner needs a vcpkg build whose MSVC import libraries don't line up with the mingw gcc cgo uses.
2.
TestCreateAddrInetV4/V6failed on Linux arm64Both fail on master under
GOOS=linux GOARCH=arm64and pass everywhere else. The cause is in the test, not inCreateAddrInet: the assertions comparedsa_dataagainst negativeint8literals, which only works where C'scharis signed. It is signed on x86-64 and on darwin, and unsigned on Linux arm64, so the same correct bytes compared unequal.The assertions now use
[]bytewith hex literals and comparebyte(ip.sa_data[i]), which is signedness-independent, and the failure message reports index/got/want instead of just "does not match". The library was producing the right sockaddr all along — no user on linux/arm64 was affected.Verification
TestCreateAddrInetV4/V6FAILBoth directions checked: reverting only the byte arrays reproduces the arm64 failure (
at 1: got 0x9a, want -0x66), and reverting only the import swap failsTestTestFilesTypeCheckOnWindows.Note CI cannot cover the arm64 fix — the matrix is
ubuntu-latestandmacos-latest, both amd64/darwin-arm64. It was verified locally in a linux/arm64 container.Honest limit:
GOOS=windows go vetstill cannot type-check the full package here, but now for an unrelated reason — cgo cross-compilation is unavailable, soCGO_ENABLED=0strips the cgo files andSrtSocketitself is undefined. Thex/sys/unixblocker is genuinely gone; a real Windows toolchain is needed to go further.🤖 Generated with Claude Code