[runtime] Add disconnect simulation to mocks::Channel
#1042
Conversation
b200d2e to
dcd4101
Compare
dcd4101 to
8a20176
Compare
|
Another approach we could take is to rely on the Arc's reference count. If both the sink and stream are alive, the What do you think of this approach @BrendanChou ? |
|
Another thing to note: if we took the approach of relying on the |
|
That's a reasonable point. I think the explicit boolean way is slightly better due to being more explicit. An extra two fields can be a bit annoying but the complexity is not bad since it's all contained within this single file. Overall I would leave it as-is |
runtime/src/mocks.rs
Outdated
| #[test] | ||
| fn test_recv_error() { | ||
| let (sink, mut stream) = Channel::init(); | ||
| let executor = deterministic::Runner::default(); | ||
|
|
||
| // If the oneshot sender is dropped before the oneshot receiver is resolved, | ||
| // the recv function should return an error. | ||
| executor.start(|_| async move { | ||
| let (v, _) = join!(stream.recv(vec![0; 5]), async { | ||
| // Take the waiter and drop it. | ||
| sink.channel.lock().unwrap().waiter.take(); | ||
| },); | ||
| assert!(matches!(v, Err(Error::RecvFailed))); | ||
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_send_error() { | ||
| let (mut sink, mut stream) = Channel::init(); | ||
| let executor = deterministic::Runner::default(); | ||
|
|
||
| // If the waiter value has a min, but the oneshot receiver is dropped, | ||
| // the send function should return an error when attempting to send the data. | ||
| executor.start(|context| async move { | ||
| // Create a waiter using a recv call. | ||
| // But then drop the receiver. | ||
| select! { | ||
| v = stream.recv( vec![0;5]) => { | ||
| panic!("unexpected value: {:?}", v); | ||
| }, | ||
| _ = context.sleep(Duration::from_millis(100)) => { | ||
| "timeout" | ||
| }, | ||
| }; | ||
| drop(stream); | ||
|
|
||
| // Try to send a message (longer than the requested amount), but the receiver is dropped. | ||
| let result = sink.send(b"hello world".to_vec()).await; | ||
| assert!(matches!(result, Err(Error::Closed))); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
the new tests make these redundant
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #1042 +/- ##
==========================================
+ Coverage 90.99% 91.05% +0.05%
==========================================
Files 191 191
Lines 54232 54427 +195
==========================================
+ Hits 49347 49557 +210
+ Misses 4885 4870 -15
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Fix #1041
Also small non-runtime changes due to this fix:
codec.rsto demonstrate a test that would have previously failed (previously would panic withruntime stalled)handshake.rsthat was previously dropping stream/sink variables. This wasn't causing an issue before, but now simulates a network disconnection so we don't want to drop these variables