test(e2e): port remaining wrapper tests from the interop repo - #4077
Open
at0m1x19 wants to merge 4 commits into
Open
test(e2e): port remaining wrapper tests from the interop repo#4077at0m1x19 wants to merge 4 commits into
at0m1x19 wants to merge 4 commits into
Conversation
at0m1x19
requested review from
AYAHASSAN287,
Ivansete-status,
NagyZoltanPeter,
darshankabariya,
fbarbu15 and
igor-sirotin
July 29, 2026 13:23
|
You can find the images built from this PR at Built from 7d6df6f |
The wrapper suite that moved into tests-e2e (#4027) was a reworked subset of the one still living in logos-delivery-interop-tests. Comparing both sides showed 21 tests here against 46 there, with no overlap in the delta: the 25 missing tests cover scenarios the reworked set never included. Ports those 25 tests, bringing the in-repo suite to the full 46: - 12 send scenarios: s01 (nil/destroyed handle), s03, s04, s05, s11, s13, s16, s18 (both orderings), s25, s29 - 7 channel lifecycle tests (rc01-rc04) - 6 wrapper corner cases: auto port allocation, MyBoundPorts, ENR Supporting changes the ported tests need: - wrapper_helpers: get_node_tcp_port, get_node_bound_ports, enr_udp_port - WrapperManager: channel_create/send/close, destroy_keep_ctx - vendored binding refreshed to the revision exposing the channel API (additive only; cffi resolves symbols lazily, so nothing existing moves) Two Edge senders were fixed while porting. build_node_config defaults relay and store to True, and the flat-JSON config path applies mode=Edge before explicit fields, so those defaults win: the Edge nodes in s11/s16/s25 came up as relay and store servers and exercised the relay path instead of lightpush. They now set relay=False and store=False, matching test_send_e2e_part2. s16 also dropped lightpush=True, which mounts the lightpush server and fails node start once relay is off; the lightpush client mounts unconditionally. Suite goes from 21 to 46 functions (53 collected). The docker subset grows from 3 to 5 as s11 and s25 need a store peer. Local run against a freshly built library: 45 passed, 2 skipped, 1 xfailed.
at0m1x19
force-pushed
the
port-wrapper-tests-delta
branch
from
August 1, 2026 08:05
27b6b04 to
43ea90a
Compare
AYAHASSAN287
approved these changes
Aug 3, 2026
channel_create subscribes to the channel's content topic since #4081, and resolving that topic to a shard needs autosharding. build_node_config leaves numShardsInNetwork at 0 and cluster 198 has no preset, so these nodes came up with static sharding and every channel_create failed with "autosharding is not configured; pass an explicit shard". Adds numShardsInNetwork=1 to the six tests that create a channel, matching what every other wrapper test that touches the send or channel API already does. rc02 is left alone: channel_send rejects on the id lookup before any shard is resolved. Verified locally against a fresh build: the five tests that complete now pass and the error string is gone from the run.
Brings in #4082 (breaking: nim-ffi 0.3.0 typed C ABI) and #4109 (per-node Persistency). The latter fixes the SIGSEGV that had been killing send-api-e2e-tests on this PR with exit 139. wrapper.py conflicted: master rewrote the vendored binding for the typed ABI but dropped the channel API this branch had added. Resolved by taking master's version as the base and re-implementing channel_create / channel_send / channel_close and destroy_keep_ctx against the new shape, with struct layouts copied from the build-generated library/generated/logosdelivery.h. Two fixups a clean auto-merge left dangling: master deleted get_node_info_raw and fixed only its own caller, so wrapper_helpers.py:219 and test_wrapper_corner_cases.py:128 now call get_node_info, which returns the raw string both callers already expected. Full non-docker suite against a library built from the merged tree: 45 passed, 2 skipped, 1 xfailed.
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
3 participants
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.
What this changes
The wrapper suite that landed in
tests-e2e/with #4027 was a reworked subset of the suite still living inlogos-delivery-interop-tests. Comparing both sides showed 21 tests here against 46 there. The 25-test difference is not duplication — those scenarios simply never made it into the reworked set, so they would have been lost once the interop repo is retired.This ports the missing 25, bringing the in-repo suite to the full 46 test functions (53 collected items).
What was ported
s01(nil / destroyed handle),s03,s04,s05,s11,s13,s16,s18(both orderings),s25,s29rc01–rc04: duplicate create, send/close on unknown channel, empty payload, ephemeral send, distinct handlesPort(0)),MyBoundPorts, ENR advertises the bound discv5 portScenarios already reworked in
test_send_e2e_part1/part2.pywere deliberately not re-copied, so there is no duplicate coverage.Supporting changes the ported tests need
src/node/wrapper_helpers.py—get_node_tcp_port,get_node_bound_ports,enr_udp_portsrc/node/wrappers_manager.py—channel_create/channel_send/channel_close,destroy_keep_ctxvendor/.../waku/wrapper.py— refreshed to the binding revision that exposes the channel APIThe binding refresh is additive only (130 insertions, 0 deletions). cffi resolves symbols lazily on first attribute access, so a library/binding version drift can only break the channel tests — it cannot fail at import time and take the rest of the suite with it. The C symbols (
logosdelivery_channel_create/send/close) are present inlibrary/channels_api/channel_api.nimand exported by the built library.Two Edge senders were misconfigured and are fixed here
Worth a close look, since this is the kind of bug that hides rather than fails:
build_node_config()defaults bothrelayandstoretoTrue, and the flat-JSON config path appliesmode=Edgebefore explicit fields (applyModethen "let explicit fields override"). Those defaults therefore win overmode="Edge". The Edge senders ins11/s16/s25came up as full relay nodes and store servers, so they exercised the relay path while claiming to test lightpush — passing for the wrong reason.They now set
relay=Falseandstore=False, matching the convention already used intest_send_e2e_part2.py.s16additionally droppedlightpush=True: that flag mounts the lightpush server, which requires relay and fails node startup (MountWithoutRelayError) once relay is off. The lightpush client is mounted unconditionally, so the sender still works.