Not sure you want this, but proposing anyway ;D
A small Python test rig under tools/nwc_protocol_test/ that exercises
the multiplayer wire protocol in src/simutrans/network/network_cmd*.cc
end-to-end. Each test spawns a headless server (its own temp
userdir, free port), sends one hand-rolled packet over TCP, and
asserts on the parsed reply.
Two overlapping purposes:
* Pin the on-wire layout (NWC_AUTH_PLAYER request/reply byte-exact,
NWC_TOOL / NWC_SERVICE request shape) so a refactor that drops
or reorders a field breaks CI instead of silently shifting bits.
* Trip on regressions in the network security class. Three
representative tests:
- test_auth_player.test_null_slot
NWC_AUTH_PLAYER targeting an unfilled player slot (the
starter map leaves 2..14 empty) must return the
wrong-password reply shape, not dereference a NULL slot
inside welt->get_player(...)->access_password_hash().
- test_tool.test_forged_client_id_does_not_crash
Pre-auth NWC_TOOL with our_client_id=0xFFFFFFFF must be
absorbed; the server must not feed wire bytes straight
into socket_list_t::get_client and trip vector_tpl
bounds.
- test_service.test_get_client_list_huge_count_does_not_crash
test_service.test_get_black_list_huge_count_does_not_crash
Pre-auth NWC_SERVICE with a forged count=0xFFFFFFFF on
SRVC_GET_CLIENT_LIST / SRVC_GET_BLACK_LIST must not drive
socket_list_t::rdwr / address_list_t::rdwr through a
billions-of-allocations loop before the admin-auth gate
rejects. (This one already passes on master after the
admin-tool/server gate landed.)
Layout, one file per NWC_* command:
wire.py Server context manager, 6-byte
(size, version, id) framing helpers,
send/recv primitives, assert_heartbeat for
the "silently absorbs and keeps ticking"
assertion.
test_auth_player.py NWC_AUTH_PLAYER.
test_tool.py NWC_TOOL.
test_service.py NWC_SERVICE.
Plain unittest, no third-party deps. Runs via
python3 -m unittest discover -v -s tools/nwc_protocol_test -t .
from the repo root and is wired into .github/workflows/run-tests.yml
under the same ASAN/UBSAN config as the scenario suite, so a
sanitizer hit on any forged packet fails the job loudly. Requires
a built simutrans binary, a pak64-compatible pakset under
simutrans/pak/, and tests/empty-16x16.sve.
While I am not the greatest fan of python, more tests are certainly fine.
Actually I don't believe that "more tests are certainly fine". There is an illness some projects suffer I call "test spamming" - it is a case where the maintenance burden and mental capacity to grasp the test suite overtakes the benefits. Sometimes tests are too low-level, too-unit-testy. And also there is a stigma, a trend that forces developers to believe all tests are good. My loose thoughts on the topic of testing.
I think the change I proposed is good. But there are other ways to do that, for example I have experimented with libfuzz - its "seed corpus" acts like a set of regression tests.
Given that simutrans has so many chances of being broken by simple changes, the test suit introduced was a big help to avoid such things. Obviously, testing is just a subset, and yes, on two occasions, fixing the tests was a little work. So far, the squirrel tests are nice.