[tmpnet] Ensure tmpnet compatibility with windows #3002
Conversation
scripts/build_test.sh
Outdated
| GOOS=$(go env GOOS) | ||
| if [[ "$GOOS" == "windows" ]]; then | ||
| # tmpnet is not compatible with windows | ||
| EXCLUDED_TARGETS="${EXCLUDED_TARGETS} | grep -v tests/fixture" | ||
| fi | ||
|
|
There was a problem hiding this comment.
I guess now tmpnet is windows compatible
There was a problem hiding this comment.
Compatible with test discovery, yes. But since we don't run e2e on windows I have no confidence in the compatibility for actually running tempory networks on windows.
There was a problem hiding this comment.
I've updated the windows implementation to panic. The previous code was untested and I don't think there's value in maintaining an untested solution.
| @@ -0,0 +1,17 @@ | |||
| // Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. | |||
There was a problem hiding this comment.
nit: should this file be renamed something like "detached_process_default.go"?
Windows seems to be the special case we have.
marun
force-pushed
the
tmpnet-windows-compatible
branch
from
af01132
to
0b8982d
Compare
This avoids breaking unit test discovery for any package that imports tmpnet.
marun
force-pushed
the
tmpnet-windows-compatible
branch
from
f82c711
to
da51a7f
Compare
|
Rebased |
| cmd.SysProcAttr = &syscall.SysProcAttr{ | ||
| Setsid: true, | ||
| } | ||
| configureDetachedProcess(cmd) |
There was a problem hiding this comment.
To clarify - this PR is to make it so that this code can compile on windows, but there is no expectation for it to run on windows?
Co-authored-by: Stephen Buttolph <stephen@avalabs.org> Signed-off-by: marun <maru.newby@avalabs.org>
Why this should be merged
This avoids breaking unit test discovery on windows for any package that imports tmpnet.
How this works
Separate os-specific process configuration into files that are conditionally compiled.
How this was tested
CI should be sufficient