Skip to content

Commit 8740113

Browse files
committed
[bfops/wasm-test]: diff-explain.txt
1 parent ecfd8d0 commit 8740113

1 file changed

Lines changed: 48 additions & 0 deletions

File tree

diff-explain.txt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
Mechanical only:
2+
3+
- sdks/rust/tests/test.rs:1 is mostly runner/config wiring so the same declared tests can execute either the native client or the wasm client.
4+
- The bulk of the async churn in sdks/rust/tests/test-client/src/main.rs:41 is helper-level: shared dispatch, shared connect*, shared wait_for_all, wasm/native shims.
5+
- sdks/rust/tests/connect_disconnect_client/src/main.rs:1 was similarly moved onto the shared async harness path, but its assertions did not change.
6+
7+
The test-body changes that are actually substantive are these:
8+
9+
- sdks/rust/tests/test-client/src/main.rs:1045 and sdks/rust/tests/test-client/src/main.rs:1347
10+
Reason: Timestamp::now() is stubbed/unimplemented on wasm32-unknown-unknown.
11+
Why the change is acceptable: those tests are not trying to validate wall-clock acquisition; they are validating timestamp serialization/round-trip through reducers/subscriptions. A fixed timestamp preserves that assertion and
12+
removes a wasm-only runtime failure.
13+
- sdks/rust/tests/test-client/src/main.rs:2179
14+
Reason 1: it now subscribes only to one_u_8 and one_u_16 instead of subscribe_all.
15+
Reason 2: it now marks the subscription as “ready” only after the relevant callbacks are installed.
16+
Why the change is needed: this test only asserts on those two tables, so subscribe_all was unnecessary setup cost. More importantly, the old ordering could report “subscription applied” before the handlers under test were
17+
actually registered, which is a real race on wasm.
18+
19+
If you are looking at my current local changes on top of that branch, there are a few additional meaningful test-body fixes:
20+
21+
- sdks/rust/tests/test-client/src/main.rs:2729
22+
Reason: it registered TestCounter expectations but never waited for them.
23+
Why the change is needed: the old version could fall off the end early on native and became an outright hang on wasm. Adding the wait does not broaden the test; it makes the existing assertion actually execute.
24+
- sdks/rust/tests/test-client/src/main.rs:2821
25+
Reason: it used a blocking Barrier inside callback-driven code.
26+
Why the change is needed: that deadlocks wasm, because the callbacks and the wait share the same JS event loop. The atomic gate preserves the intended semantics: insert exactly once, only after all three subscriptions are
27+
ready.
28+
- sdks/rust/tests/test-client/src/main.rs:2919
29+
Reason: it used a second TestCounter as a cross-client blocking barrier.
30+
Why the change is needed: same wasm deadlock issue. The test’s actual assertion is per-client filtering after that client’s own subscription applies; it does not require both clients to reach a global barrier first.
31+
- sdks/rust/tests/test-client/src/main.rs:2977
32+
Reason: same pattern as test_parameterized_subscription.
33+
Why the change is needed: the barrier was not part of the behavior under test, only a synchronization convenience that is invalid on wasm. Removing it keeps the test focused on RLS visibility rather than cross-client
34+
simultaneity.
35+
36+
So the short version is:
37+
38+
- Most of the scary-looking diff is harness plumbing, not changed assertions.
39+
- The real test-content changes were either:
40+
- replacing native-only time acquisition with a deterministic value, or
41+
- removing synchronization patterns that were accidentally relying on native’s threaded runtime and were invalid on wasm.
42+
43+
If you want, I can turn this into a reviewer-oriented checklist grouped as:
44+
45+
1. pure harness changes,
46+
2. assertion-preserving test fixes,
47+
3. true behavioral test changes.
48+

0 commit comments

Comments
 (0)