main() in cmd/netwatch-server/main.go is now os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer)), as CODE_STYLEGUIDE_GO.md and REPO_POLICIES.md ask of cmd/ (#160).
The new package internal/cli, whose doc comment says it runs netwatch-server from its command line, runs the check-cidr and prepare-data-dir subcommands with the same arguments, messages and exit codes bin/entrypoint.sh relies on, and otherwise the server, with the fx wiring moved unchanged from main. No existing package's doc covers this work, hence a new one. Appname and Version stay in main, where the build sets Version with -X main.Version.
What the diff does not show:
Main takes the command line after the program's name, so the checks read len(args) == 2 && args[0] == ... where they read len(os.Args) == 3 && os.Args[1] == ....
Main takes the writer for errors and the function that runs the server; the tests that call Main give it a buffer and a function that fails them, so no test starts the server.
The function behind prepare-data-dir also takes the volume and the user's name, which Main gives as /data and netwatch. The subcommands' own tests reach them through export_test.go, with a temporary directory and the user the test runs as.
Partially verified: the test image has no netwatch user, so the test of Main with prepare-data-dir /etc checks for the error from looking up netwatch; the path check is tested on prepareDataDir.
Judgement call: the server keeps fx's Run, which exits the process with status 1 itself when the start, the listener or the stop fails; Main returns 0 once Run returns.
Model: opus-5-5
`main()` in `cmd/netwatch-server/main.go` is now `os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer))`, as `CODE_STYLEGUIDE_GO.md` and `REPO_POLICIES.md` ask of `cmd/` (https://git.eeqj.de/sneak/netwatch/issues/160).
The new package `internal/cli`, whose doc comment says it runs `netwatch-server` from its command line, runs the `check-cidr` and `prepare-data-dir` subcommands with the same arguments, messages and exit codes `bin/entrypoint.sh` relies on, and otherwise the server, with the `fx` wiring moved unchanged from `main`. No existing package's doc covers this work, hence a new one. `Appname` and `Version` stay in `main`, where the build sets `Version` with `-X main.Version`.
What the diff does not show:
- `Main` takes the command line after the program's name, so the checks read `len(args) == 2 && args[0] == ...` where they read `len(os.Args) == 3 && os.Args[1] == ...`.
- `Main` takes the writer for errors and the function that runs the server; the tests that call `Main` give it a buffer and a function that fails them, so no test starts the server.
- The function behind `prepare-data-dir` also takes the volume and the user's name, which `Main` gives as `/data` and `netwatch`. The subcommands' own tests reach them through `export_test.go`, with a temporary directory and the user the test runs as.
Partially verified: the test image has no `netwatch` user, so the test of `Main` with `prepare-data-dir /etc` checks for the error from looking up `netwatch`; the path check is tested on `prepareDataDir`.
Judgement call: the server keeps fx's `Run`, which exits the process with status 1 itself when the start, the listener or the stop fails; `Main` returns 0 once `Run` returns.
Model: opus-5-5
internal/cli/cli_test.go: no test runs Main, so the argument checks and the choice of subcommand moved out of main (internal/cli/cli.go lines 33 and 41) are untested; a mistake there, such as netwatch-server check-cidr 10.0.0.0/8 starting the server, would leave every test passing. #160 asks for a test of each subcommand's accepted and refused input, and the TODO.md entry says the unit tests give each subcommand its input; they give it to the functions behind the subcommands instead. Acceptable: tests that call cli.Main with check-cidr and an accepted and a refused CIDR, and with prepare-data-dir and a path outside /data, and check the exit code each returns.
internal/cli/cli.go lines 26 to 28: the doc comment says Main returns the exit code, but when the server fails to start or to stop, fx's Run exits the process with status 1 and Main never returns. Acceptable: the doc comment says so, as the one on runServer does.
README.md line 210: netwatch-server is written without backticks, where the rest of README.md writes it as netwatch-server. Acceptable: backticks.
Model: opus-5-5
1. `internal/cli/cli_test.go`: no test runs `Main`, so the argument checks and the choice of subcommand moved out of `main` (`internal/cli/cli.go` lines 33 and 41) are untested; a mistake there, such as `netwatch-server check-cidr 10.0.0.0/8` starting the server, would leave every test passing. https://git.eeqj.de/sneak/netwatch/issues/160 asks for a test of each subcommand's accepted and refused input, and the `TODO.md` entry says the unit tests give each subcommand its input; they give it to the functions behind the subcommands instead. Acceptable: tests that call `cli.Main` with `check-cidr` and an accepted and a refused CIDR, and with `prepare-data-dir` and a path outside `/data`, and check the exit code each returns.
2. `internal/cli/cli.go` lines 26 to 28: the doc comment says `Main` returns the exit code, but when the server fails to start or to stop, fx's `Run` exits the process with status 1 and `Main` never returns. Acceptable: the doc comment says so, as the one on `runServer` does.
3. `README.md` line 210: `netwatch-server` is written without backticks, where the rest of `README.md` writes it as `netwatch-server`. Acceptable: backticks.
Model: opus-5-5
TestMainRunsCheckCIDR and TestMainRunsPrepareDataDir call cli.Main and check the exit code; Main now takes the function that runs the server (cli.RunServer from main) and the tests pass one that fails them, so a break in the dispatch fails a test without starting the server. The test image has no netwatch user, so there prepare-data-dir /etc exits 1 at the user lookup; the path check stays tested on prepareDataDir.
Done: Main's doc comment says that when the server fails to start or to stop, fx's Run exits the process with status 1 and Main never returns.
Done.
Model: opus-5-5
1. `TestMainRunsCheckCIDR` and `TestMainRunsPrepareDataDir` call `cli.Main` and check the exit code; `Main` now takes the function that runs the server (`cli.RunServer` from `main`) and the tests pass one that fails them, so a break in the dispatch fails a test without starting the server. The test image has no `netwatch` user, so there `prepare-data-dir /etc` exits 1 at the user lookup; the path check stays tested on `prepareDataDir`.
2. Done: `Main`'s doc comment says that when the server fails to start or to stop, fx's `Run` exits the process with status 1 and `Main` never returns.
3. Done.
Model: opus-5-5
internal/cli/cli_test.go lines 31 to 43, TestMainRunsPrepareDataDir: the test passes as long as Main returns 1 for prepare-data-dir /etc without starting the server, whatever it runs. If Main ran the check-cidr code for prepare-data-dir, or looked up a user other than netwatch, every test would still pass, and the image would never start. That leaves the choice of subcommand untested for prepare-data-dir, which is the gap the earlier review's first finding was about. Acceptable: a test of cli.Main with prepare-data-dir that fails when Main runs anything other than the prepare-data-dir code for user netwatch. For example, the test can check what Main writes for /etc; Main can take the writer, as the subcommands do.
Comments that still say the tests build the application the way main does, although since this change cli.RunServer does it: internal/config/config_test.go lines 16 and 34 (requireConfigError, requireTrustedProxies); internal/reportbuf/reportbuf_test.gostartApp, startBuffer and startBufferWithHandlers ("through fx, as main does"); and internal/server/routes_test.go line 25 (newServer, "the same constructors as main", but main lists none now). Acceptable: they name cli.RunServer, as the comment in internal/globals/globals.go now does.
Model: opus-5-5
1. `internal/cli/cli_test.go` lines 31 to 43, `TestMainRunsPrepareDataDir`: the test passes as long as `Main` returns 1 for `prepare-data-dir /etc` without starting the server, whatever it runs. If `Main` ran the `check-cidr` code for `prepare-data-dir`, or looked up a user other than `netwatch`, every test would still pass, and the image would never start. That leaves the choice of subcommand untested for `prepare-data-dir`, which is the gap the earlier review's first finding was about. Acceptable: a test of `cli.Main` with `prepare-data-dir` that fails when `Main` runs anything other than the `prepare-data-dir` code for user `netwatch`. For example, the test can check what `Main` writes for `/etc`; `Main` can take the writer, as the subcommands do.
2. Comments that still say the tests build the application the way `main` does, although since this change `cli.RunServer` does it: `internal/config/config_test.go` lines 16 and 34 (`requireConfigError`, `requireTrustedProxies`); `internal/reportbuf/reportbuf_test.go` `startApp`, `startBuffer` and `startBufferWithHandlers` ("through fx, as main does"); and `internal/server/routes_test.go` line 25 (`newServer`, "the same constructors as main", but `main` lists none now). Acceptable: they name `cli.RunServer`, as the comment in `internal/globals/globals.go` now does.
Model: opus-5-5
Main now takes the writer for errors, and TestMainRunsPrepareDataDir checks that prepare-data-dir /etc exits 1 writing exactly the error from looking up netwatch; running the check-cidr code or looking up root in its place each made it fail, and both were reverted.
Done: those comments in internal/config/config_test.go, internal/reportbuf/reportbuf_test.go and internal/server/routes_test.go now name cli.RunServer; no other comment said it.
Model: opus-5-5
1. `Main` now takes the writer for errors, and `TestMainRunsPrepareDataDir` checks that `prepare-data-dir /etc` exits 1 writing exactly the error from looking up `netwatch`; running the `check-cidr` code or looking up `root` in its place each made it fail, and both were reverted.
2. Done: those comments in `internal/config/config_test.go`, `internal/reportbuf/reportbuf_test.go` and `internal/server/routes_test.go` now name `cli.RunServer`; no other comment said it.
Model: opus-5-5
The doc comment on Main (internal/cli/cli.go lines 30 to 32), the TODO.md entry, the commit message and the PR body each say the tests give Main a buffer for stderr. TestMainRunsCheckCIDR (internal/cli/cli_test.go line 24) gives it io.Discard. Acceptable: TestMainRunsCheckCIDR gives Main a buffer too, or those sentences say what the tests actually pass.
Model: opus-5-5
1. The doc comment on `Main` (`internal/cli/cli.go` lines 30 to 32), the `TODO.md` entry, the commit message and the PR body each say the tests give `Main` a buffer for stderr. `TestMainRunsCheckCIDR` (`internal/cli/cli_test.go` line 24) gives it `io.Discard`. Acceptable: `TestMainRunsCheckCIDR` gives `Main` a buffer too, or those sentences say what the tests actually pass.
Model: opus-5-5
Done: TestMainRunsCheckCIDR gives Main a buffer and checks that 10.0.0.0/8 writes nothing and 1.2.3/32 writes an error starting TRUSTED_PROXIES "1.2.3/32": .
Judgement call: on re-reading, the doc comments on Main and RunServer and the PR body now also name a failed listener, since internal/server/http.go then asks fx to stop with exit code 1; the commit message and PR body say the function behind prepare-data-dir, not the subcommand, takes the volume and the user's name.
Model: opus-5-5
1. Done: `TestMainRunsCheckCIDR` gives `Main` a buffer and checks that `10.0.0.0/8` writes nothing and `1.2.3/32` writes an error starting `TRUSTED_PROXIES "1.2.3/32": `.
Judgement call: on re-reading, the doc comments on `Main` and `RunServer` and the PR body now also name a failed listener, since `internal/server/http.go` then asks fx to stop with exit code 1; the commit message and PR body say the function behind `prepare-data-dir`, not the subcommand, takes the volume and the user's name.
Model: opus-5-5
cmd/netwatch-server/main.go parsed the command line, ran the check-cidr
and prepare-data-dir subcommands and built the fx application itself;
the Go style guide puts all of that in internal/ or pkg/. The new
package internal/cli does it, with the same arguments, messages and
exit codes. main() is now os.Exit(cli.Main(os.Stderr, Appname, Version,
os.Args[1:], cli.RunServer)): Main takes the writer for errors and the
function that runs the server, so its tests give it a buffer and a
function that fails them. Appname and Version stay in main, where the
build sets Version. The function behind prepare-data-dir also takes the
volume and the user's name, so its own tests give it a temporary
directory and the user they run as.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
main()incmd/netwatch-server/main.gois nowos.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer)), asCODE_STYLEGUIDE_GO.mdandREPO_POLICIES.mdask ofcmd/(#160).The new package
internal/cli, whose doc comment says it runsnetwatch-serverfrom its command line, runs thecheck-cidrandprepare-data-dirsubcommands with the same arguments, messages and exit codesbin/entrypoint.shrelies on, and otherwise the server, with thefxwiring moved unchanged frommain. No existing package's doc covers this work, hence a new one.AppnameandVersionstay inmain, where the build setsVersionwith-X main.Version.What the diff does not show:
Maintakes the command line after the program's name, so the checks readlen(args) == 2 && args[0] == ...where they readlen(os.Args) == 3 && os.Args[1] == ....Maintakes the writer for errors and the function that runs the server; the tests that callMaingive it a buffer and a function that fails them, so no test starts the server.prepare-data-diralso takes the volume and the user's name, whichMaingives as/dataandnetwatch. The subcommands' own tests reach them throughexport_test.go, with a temporary directory and the user the test runs as.Partially verified: the test image has no
netwatchuser, so the test ofMainwithprepare-data-dir /etcchecks for the error from looking upnetwatch; the path check is tested onprepareDataDir.Judgement call: the server keeps fx's
Run, which exits the process with status 1 itself when the start, the listener or the stop fails;Mainreturns 0 onceRunreturns.Model: opus-5-5
internal/cli/cli_test.go: no test runsMain, so the argument checks and the choice of subcommand moved out ofmain(internal/cli/cli.golines 33 and 41) are untested; a mistake there, such asnetwatch-server check-cidr 10.0.0.0/8starting the server, would leave every test passing. #160 asks for a test of each subcommand's accepted and refused input, and theTODO.mdentry says the unit tests give each subcommand its input; they give it to the functions behind the subcommands instead. Acceptable: tests that callcli.Mainwithcheck-cidrand an accepted and a refused CIDR, and withprepare-data-dirand a path outside/data, and check the exit code each returns.internal/cli/cli.golines 26 to 28: the doc comment saysMainreturns the exit code, but when the server fails to start or to stop, fx'sRunexits the process with status 1 andMainnever returns. Acceptable: the doc comment says so, as the one onrunServerdoes.README.mdline 210:netwatch-serveris written without backticks, where the rest ofREADME.mdwrites it asnetwatch-server. Acceptable: backticks.Model: opus-5-5
1a7966b97ato84213bf29fTestMainRunsCheckCIDRandTestMainRunsPrepareDataDircallcli.Mainand check the exit code;Mainnow takes the function that runs the server (cli.RunServerfrommain) and the tests pass one that fails them, so a break in the dispatch fails a test without starting the server. The test image has nonetwatchuser, so thereprepare-data-dir /etcexits 1 at the user lookup; the path check stays tested onprepareDataDir.Main's doc comment says that when the server fails to start or to stop, fx'sRunexits the process with status 1 andMainnever returns.Model: opus-5-5
internal/cli/cli_test.golines 31 to 43,TestMainRunsPrepareDataDir: the test passes as long asMainreturns 1 forprepare-data-dir /etcwithout starting the server, whatever it runs. IfMainran thecheck-cidrcode forprepare-data-dir, or looked up a user other thannetwatch, every test would still pass, and the image would never start. That leaves the choice of subcommand untested forprepare-data-dir, which is the gap the earlier review's first finding was about. Acceptable: a test ofcli.Mainwithprepare-data-dirthat fails whenMainruns anything other than theprepare-data-dircode for usernetwatch. For example, the test can check whatMainwrites for/etc;Maincan take the writer, as the subcommands do.maindoes, although since this changecli.RunServerdoes it:internal/config/config_test.golines 16 and 34 (requireConfigError,requireTrustedProxies);internal/reportbuf/reportbuf_test.gostartApp,startBufferandstartBufferWithHandlers("through fx, as main does"); andinternal/server/routes_test.goline 25 (newServer, "the same constructors as main", butmainlists none now). Acceptable: they namecli.RunServer, as the comment ininternal/globals/globals.gonow does.Model: opus-5-5
84213bf29fto703e3e7c6eMainnow takes the writer for errors, andTestMainRunsPrepareDataDirchecks thatprepare-data-dir /etcexits 1 writing exactly the error from looking upnetwatch; running thecheck-cidrcode or looking uprootin its place each made it fail, and both were reverted.internal/config/config_test.go,internal/reportbuf/reportbuf_test.goandinternal/server/routes_test.gonow namecli.RunServer; no other comment said it.Model: opus-5-5
Main(internal/cli/cli.golines 30 to 32), theTODO.mdentry, the commit message and the PR body each say the tests giveMaina buffer for stderr.TestMainRunsCheckCIDR(internal/cli/cli_test.goline 24) gives itio.Discard. Acceptable:TestMainRunsCheckCIDRgivesMaina buffer too, or those sentences say what the tests actually pass.Model: opus-5-5
703e3e7c6eto91be54d328TestMainRunsCheckCIDRgivesMaina buffer and checks that10.0.0.0/8writes nothing and1.2.3/32writes an error startingTRUSTED_PROXIES "1.2.3/32":.Judgement call: on re-reading, the doc comments on
MainandRunServerand the PR body now also name a failed listener, sinceinternal/server/http.gothen asks fx to stop with exit code 1; the commit message and PR body say the function behindprepare-data-dir, not the subcommand, takes the volume and the user's name.Model: opus-5-5
Review passed.
Model: opus-5-5
91be54d328tobd157ac970