From a320817ee51a72f40f7d1deb60e8faa40977a607 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 1 Jun 2022 14:50:14 +0200 Subject: [PATCH] test_utils.rs / TestContext: Remove poison channel, don't fail test if events loop panics (because it can't panic anymore) (#3380) I added this poison_sender and poison_receiver stuff back when we had event listeners (which were called "sinks", too, but anyway), i.e. user-definable closures that were run in the events loop. This was to make sure that the test fails if the closure panics. But since we don't have them anymore and this code isn't supposed to panic anyway: ```rust while let Some(event) = events.recv().await { for sender in senders.read().await.iter() { sender.try_send(event.clone()).ok(); } } ``` --- CHANGELOG.md | 2 +- src/test_utils.rs | 39 ++++----------------------------------- 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 25e769371..69caf4d25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased ### Changes -- refactorings #3373 #3345 +- refactorings #3373 #3345 #3380 ### Fixes - delete outgoing MDNs found in the Sent folder on Gmail #3372 diff --git a/src/test_utils.rs b/src/test_utils.rs index 8cfc937fb..0fe1d3a67 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -5,7 +5,6 @@ use std::collections::BTreeMap; use std::ops::Deref; use std::panic; -use std::thread; use std::time::{Duration, Instant}; use ansi_term::Color; @@ -144,8 +143,6 @@ pub struct TestContext { pub evtracker: EventTracker, /// Channels which should receive events from this context. event_senders: Arc>>>, - /// Receives panics from sinks ("sink" means "event handler" here) - poison_receiver: Receiver, /// Reference to implicit [`LogSink`] so it is dropped together with the context. /// /// Only used if no explicit `log_sender` is passed into [`TestContext::new_internal`] @@ -230,30 +227,13 @@ impl TestContext { let (evtracker_sender, evtracker_receiver) = channel::unbounded(); let event_senders = Arc::new(RwLock::new(vec![log_sender, evtracker_sender])); let senders = Arc::clone(&event_senders); - let (poison_sender, poison_receiver) = channel::bounded(1); task::spawn(async move { - // Make sure that the test fails if there is a panic on this thread here - // (but not if there is a panic on another thread) - let looptask_id = task::current().id(); - let orig_hook = panic::take_hook(); - panic::set_hook(Box::new(move |panic_info| { - if let Some(panicked_task) = task::try_current() { - if panicked_task.id() == looptask_id { - poison_sender.try_send(panic_info.to_string()).ok(); - } - } - orig_hook(panic_info); - })); - while let Some(event) = events.recv().await { - { - let sinks = senders.read().await; - for sender in sinks.iter() { - // Don't block because someone wanted to use a oneshot receiver, use - // an unbounded channel if you want all events. - sender.try_send(event.clone()).ok(); - } + for sender in senders.read().await.iter() { + // Don't block because someone wanted to use a oneshot receiver, use + // an unbounded channel if you want all events. + sender.try_send(event.clone()).ok(); } } }); @@ -263,7 +243,6 @@ impl TestContext { dir, evtracker: EventTracker(evtracker_receiver), event_senders, - poison_receiver, log_sink, } } @@ -617,16 +596,6 @@ impl Deref for TestContext { } } -impl Drop for TestContext { - fn drop(&mut self) { - if !thread::panicking() { - if let Ok(p) = self.poison_receiver.try_recv() { - panic!("{}", p); - } - } - } -} - /// A receiver of [`Event`]s which will log the events to the captured test stdout. /// /// Tests redirect the stdout of the test thread and capture this, showing the captured