e2e_part2#179
Conversation
* - Add S06 relay-only test case for testing message propagation without a store. - Update `wrapper_helpers` for clearer event type handling and type annotations (`Optional[...]` usage). - Simplify `get_node_multiaddr` to retrieve addresses via `get_node_info_raw`. - Refactor `wrappers_manager` to adjust bindings path to `vendor` directory and add `get_node_info_raw` method. - Update `.gitignore` to exclude `store.sqlite3*`. * Refactor S06 relay-only test: replace try-finally blocks with context managers for clarity and conciseness. * Migrate S06 relay-only test to `test_send_e2e.py` and refactor with `StepsCommon` for reusability. --------- Co-authored-by: Egor Rachkovskii <egorrachkovskii@status.im>
… helper (#178) - Refactored the `wait_for_event` function for clarity and to ensure proper deadline handling within the loop. - Introduced `assert_event_invariants` to validate per-request event properties, enforcing invariants like correct `requestId`, no duplicate terminal events, and proper timing between `Propagated` and `Sent`. - Added tests for `assert_event_invariants` enforcement in `S14` and `S15` lightpush scenarios. Co-authored-by: Egor Rachkovskii <egorrachkovskii@status.im>
* Add `assert_event_invariants` to enforce per-request event constraints and integrate into relevant tests * Integrate `assert_event_invariants` into edge and store tests * Remove redundant comments from `test_send_e2e.py` --------- Co-authored-by: Egor Rachkovskii <egorrachkovskii@status.im>
fbarbu15
left a comment
There was a problem hiding this comment.
LGTM, thanks but can you please add a full run test report to the PR description?
* Add tests for auto-subscribe on first send and isolated sender with no peers * Add PR CI workflow with tiered test strategy - pr_tests.yml: build job with cache, wrapper-tests, smoke-tests, and label-triggered full-suite - test_common.yml: add deploy_allure/send_discord inputs so PR runs skip reporting side effects - Add docker_required marker to S19 (needs Docker, excluded from wrapper-only CI job) - Register docker_required marker in pytest.ini * Document PR CI test workflows in README * Refine PR CI test strategy: - Exclude `docker_required` tests from smoke set in `pr_tests.yml`. - Add `wait_for_connected` helper for connection state checks. - Update S19 test to dynamically create and clean up the store node setup. - General simplifications and improved test stability. * Add `wait_for_connected` assertion to ensure sender connection state before propagation test * Refine tests and CI workflows: - Replace `ERROR_TIMEOUT_S` with `ERROR_AFTER_CACHE_EXPIRY_TIMEOUT_S` in `test_send_e2e.py`. - Adjust timeout assertion for better clarity and accuracy. - Update `pr_tests.yml` to add retries (`--reruns`) and ignore wrapper tests in smoke tests. - Change `test_common.yml` default Discord reporting to `false`. * Normalize `portsshift` to `portsShift` in `test_send_e2e.py` configuration definitions. --------- Co-authored-by: Egor Rachkovskii <egorrachkovskii@status.im>
Co-authored-by: Egor Rachkovskii <egorrachkovskii@status.im>
at0m1x19
left a comment
There was a problem hiding this comment.
A few comments — mostly suggestions, nothing blocking the overall direction. Thanks for the work on this!
at0m1x19
left a comment
There was a problem hiding this comment.
Follow-up batch — a question on the xfail markers, a validated note on docker_required, and a couple of unused imports.
latest job |
…htpush peer discovery.
…to broken lightpush peer discovery.
Latest run , all skipped tests failing for issues opened |
Ivansete-status
left a comment
There was a problem hiding this comment.
Sorry for late review. Just adding a few comments. Thanks!
|
|
||
| logger.debug(f"messages length is {len(self.store_response.messages)}") | ||
| assert self.store_response.messages, f"Peer {node.image} couldn't find any messages. " f"Actual response: {self.store_response.resp_json}" | ||
| assert len(self.store_response.messages) >= len(expected_hashes), ( |
There was a problem hiding this comment.
Isn't it better to assert strict equal?
There was a problem hiding this comment.
The real correctness check is the loop (assert expected_hash in actual_hashes)
Store state isn't isolated ,the store node can hold messages from prior store requests .
| "tcpPort": free_port(), | ||
| "discv5UdpPort": free_port(), | ||
| "restPort": free_port(), |
There was a problem hiding this comment.
After having merged the following, it should be enough to pass 0 to these to perform auto-port internally. Maybe another test to consider in the future: logos-messaging/logos-delivery#3828
There was a problem hiding this comment.
I will add it in recent tests , add specific test for it
| "relay": True, | ||
| "store": False, | ||
| "lightpush": False, | ||
| "filter": False, |
There was a problem hiding this comment.
Is interesting to only configure nodes by only touching the mode attribute, i.e., only setting either mode: "Edge" or mode: "Core". Even though nowadays is supported to manually set protocols, we aim for devs to not tweak that directly, in the future.
I mean, this is actually a flaw in the API, that should not accept those attributes, and we might fix that in the future.
Summary
End-to-end test coverage for the wrapper send path, plus the supporting infrastructure (helpers, CI pipeline, store assertions). Adds 19 new scenarios across two files
Tests
tests/wrappers_tests/test_send_e2e_part1.py(TestSendBeforeRelay):reliabilityEnabled=False— Propagated only, no Senttests/wrappers_tests/test_send_e2e_part2.py:New helpers
src/node/wrapper_helpers.py—EventCollector(thread-safe async event sink), event-type predicates,wait_for_propagated/sent/error/connected,assert_event_invariants,get_node_multiaddr,create_message_bindings.src/steps/store.py—check_sent_message_is_storedto assert hashes are present in store.src/node/wrappers_manager.py—get_node_info_raw(returns the unparsed string).Infra / wiring
.github/workflows/pr_tests.yml— build (cached on submodule hash) → wrapper tests → smoke; full 18-shard suite triggered byfull-testlabel.Known issues
storeClientCount=1issuejson_serialization/writer.nim → w.inObject() AssertionDefectand nwaku SIGSEGV during shutdown/churn. Note: flaky not show up alwaysNoPeersToPublishand eventually emitsmessage_error: Unable to send within retry time window.note: the main point is that it's not possible to force the miss round