test: replace the bash relay integration tests with a Python topology harness - #647
test: replace the bash relay integration tests with a Python topology harness#647afrind wants to merge 1 commit into
Conversation
mondain
left a comment
There was a problem hiding this comment.
@mondain reviewed 15 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on afrind).
michalhosna
left a comment
There was a problem hiding this comment.
Love the DSL Idea
@michalhosna reviewed 1 file and made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on afrind).
docs/dev/test-harness.md line 344 at r1 (raw file):
[`test/test_relay_lifecycle.sh`](/test/test_relay_lifecycle.sh), and `_resolve_moqbin` mirrors [`test/test_moqbin.sh`](/test/test_moqbin.sh); those pairs must stay in agreement while both the bash and Python tests exist.
Is this intended to human or agents? I feel like I am missing some short summary and pointer to here.
This is loooong.
But this is maybe out-of-scope and we should create some docs/README.md as an Table of contents / index of all the docs.
test/CMakeLists.txt line 9 at r1 (raw file):
# in the harness — a version arg to find_package is a minimum, and no CI runner # is old enough to trip over one. That needs a lint pinned to py39. find_package(Python3 3.9 COMPONENTS Interpreter REQUIRED)
Could we use uv for python management here?
I know its additional dependency, but its just a small binary, and it sidesteps a lot of python version problems. And can lock a specific python version.
Given that this is adding a considerable amount of python machinery, and we are making the python kind of 1st class support language, I think we should add some CI lints.
I highly recomend using the Astral stuff (it's fast)
- lint+code format: https://docs.astral.sh/ruff/
- https://docs.astral.sh/uv/
test/test_ports.sh line 86 at r1 (raw file):
TEST_HARNESS_CHAIN_BASE=19800 TEST_HARNESS_HOPS_BASE=19810 TEST_HARNESS_PYRAMID_BASE=19820
Why aren't we using listen on :0 where the OS picks a free port, and we don't have to worry about being unique and colliding? We only need the ability to read the port back.
This is maybe out-of-scope, and should be resolved in the issue,.
afrind
left a comment
There was a problem hiding this comment.
@afrind reviewed 4 files and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on michalhosna).
docs/dev/test-harness.md line 344 at r1 (raw file):
I actually had it write this doc and read it (in markdown viewer) so I could understand what the harness does, and used it to suggest better test APIs, and simplify the tests.
I think it's human readable if you are interested in writing or reading tests using the harness.
But this is maybe out-of-scope and we should create some docs/README.md as an Table of contents / index of all the docs.
I'll call that out of scope for now.
test/CMakeLists.txt line 9 at r1 (raw file):
Previously, michalhosna (Michal Hošna) wrote…
Could we use
uvfor python management here?I know its additional dependency, but its just a small binary, and it sidesteps a lot of python version problems. And can lock a specific python version.
Given that this is adding a considerable amount of python machinery, and we are making the python kind of 1st class support language, I think we should add some CI lints.
I highly recomend using the Astral stuff (it's fast)
- lint+code format: https://docs.astral.sh/ruff/
- https://docs.astral.sh/uv/
seems fine - will add. I used ruff locally.
test/test_ports.sh line 86 at r1 (raw file):
Previously, michalhosna (Michal Hošna) wrote…
Why aren't we using listen on
:0where the OS picks a free port, and we don't have to worry about being unique and colliding? We only need the ability to read the port back.This is maybe out-of-scope, and should be resolved in the issue,.
yes that's more scalable, but it's a separate project for now.
deab8cb to
44e5449
Compare
… harness
Each shell test carried its own process management, port arithmetic, curl|jq
polling and reaping — test_relay_chain.sh was 429 lines, and adding a topology
meant copying all of it. Some checks were silently no-ops off Linux: the port
pre-flight ran `ss -uln`, which does not exist on macOS, with stderr discarded.
test/lib/moq_harness.py takes a declared topology instead and generates the
configs, ports, waits and teardown. `upstream` may name a relay declared later,
so cycles work, and start() derives the peering waits from the topology rather
than from hand-counted session numbers.
def run(h):
h.relay("A")
h.relay("B", upstream="A")
h.start()
pub = h.actor("pub", "publisher", relay="A", ns="moq-date", track="date")
sub = h.actor("sub", "subscriber", relay="B", ns="moq-date",
track="date", timeout=2)
pub.start()
h.wait_sessions_atleast("A", "+1")
sub.run()
h.expect_received(sub)
main(run, base_port_key="chain")
Converts test_relay_chain.sh and test_relay_hops_cycle.sh, and adds
test_relay_pyramid.py for fan-out across a branching tree and containment after
a relay is killed. docs/dev/test-harness.md covers writing a new one.
44e5449 to
d34ca00
Compare
Each shell test carried its own process management, port arithmetic, curl|jq polling and reaping — test_relay_chain.sh was 429 lines, and adding a topology meant copying all of it. Some checks were silently no-ops off Linux: the port pre-flight ran
ss -uln, which does not exist on macOS, with stderr discarded.test/lib/moq_harness.py takes a declared topology instead and generates the configs, ports, waits and teardown.
upstreammay name a relay declared later, so cycles work, and start() derives the peering waits from the topology rather than from hand-counted session numbers.Converts test_relay_chain.sh and test_relay_hops_cycle.sh, and adds test_relay_pyramid.py for fan-out across a branching tree and containment after a relay is killed. docs/dev/test-harness.md covers writing a new one.
This change is