Make integration tests less flaky

- Send sigterm to Xwayland after the test has completed
- Before polling for a new event, try to just grab one from xcb

Fixes #118
This commit is contained in:
Shawn Wallace 2025-03-13 00:47:40 -04:00
parent caf88fcb45
commit f459c604f5
2 changed files with 32 additions and 25 deletions

View file

@ -41,7 +41,7 @@ pub trait RunData {
} }
fn created_server(&self) {} fn created_server(&self) {}
fn connected_server(&self) {} fn connected_server(&self) {}
fn xwayland_ready(&self, _display: String) {} fn xwayland_ready(&self, _display: String, _pid: u32) {}
} }
pub fn main(data: impl RunData) -> Option<()> { pub fn main(data: impl RunData) -> Option<()> {
@ -75,6 +75,8 @@ pub fn main(data: impl RunData) -> Option<()> {
.spawn() .spawn()
.unwrap(); .unwrap();
let xwl_pid = xwayland.id();
let (mut finish_tx, mut finish_rx) = UnixStream::pair().unwrap(); let (mut finish_tx, mut finish_rx) = UnixStream::pair().unwrap();
let stderr = xwayland.stderr.take().unwrap(); let stderr = xwayland.stderr.take().unwrap();
std::thread::spawn(move || { std::thread::spawn(move || {
@ -151,7 +153,7 @@ pub fn main(data: impl RunData) -> Option<()> {
display.pop(); display.pop();
display.insert(0, ':'); display.insert(0, ':');
info!("Connected to Xwayland on {display}"); info!("Connected to Xwayland on {display}");
data.xwayland_ready(display); data.xwayland_ready(display, xwl_pid);
xstate.server_state_setup(&mut server_state); xstate.server_state_setup(&mut server_state);
#[cfg(feature = "systemd")] #[cfg(feature = "systemd")]

View file

@ -1,4 +1,5 @@
use rustix::event::{poll, PollFd, PollFlags}; use rustix::event::{poll, PollFd, PollFlags};
use rustix::process::{Pid, Signal, WaitOptions};
use std::mem::ManuallyDrop; use std::mem::ManuallyDrop;
use std::os::fd::{AsRawFd, BorrowedFd}; use std::os::fd::{AsRawFd, BorrowedFd};
use std::os::unix::net::UnixStream; use std::os::unix::net::UnixStream;
@ -20,6 +21,7 @@ struct TestDataInner {
server_connected: AtomicBool, server_connected: AtomicBool,
display: Mutex<Option<String>>, display: Mutex<Option<String>>,
server: Mutex<Option<UnixStream>>, server: Mutex<Option<UnixStream>>,
pid: Mutex<Option<u32>>,
} }
#[derive(Default, Clone)] #[derive(Default, Clone)]
@ -50,8 +52,9 @@ impl xwls::RunData for TestData {
self.server_connected.store(true, Ordering::Relaxed); self.server_connected.store(true, Ordering::Relaxed);
} }
fn xwayland_ready(&self, display: String) { fn xwayland_ready(&self, display: String, pid: u32) {
*self.display.lock().unwrap() = Some(display); *self.display.lock().unwrap() = Some(display);
*self.pid.lock().unwrap() = Some(pid);
} }
fn display(&self) -> Option<&str> { fn display(&self) -> Option<&str> {
@ -70,6 +73,7 @@ struct Fixture {
thread: ManuallyDrop<JoinHandle<Option<()>>>, thread: ManuallyDrop<JoinHandle<Option<()>>>,
pollfd: PollFd<'static>, pollfd: PollFd<'static>,
display: String, display: String,
pid: Pid,
} }
impl Drop for Fixture { impl Drop for Fixture {
@ -77,6 +81,9 @@ impl Drop for Fixture {
let thread = unsafe { ManuallyDrop::take(&mut self.thread) }; let thread = unsafe { ManuallyDrop::take(&mut self.thread) };
if thread.is_finished() { if thread.is_finished() {
thread.join().expect("Main thread panicked"); thread.join().expect("Main thread panicked");
} else {
rustix::process::kill_process(self.pid, Signal::Term).unwrap();
rustix::process::waitpid(Some(self.pid), WaitOptions::NOHANG).unwrap();
} }
} }
} }
@ -141,11 +148,13 @@ impl Fixture {
assert!(ready, "connecting to xwayland failed"); assert!(ready, "connecting to xwayland failed");
let display = our_data.display.lock().unwrap().take().unwrap(); let display = our_data.display.lock().unwrap().take().unwrap();
let pid = our_data.pid.lock().unwrap().take().unwrap();
Self { Self {
testwl, testwl,
thread: ManuallyDrop::new(thread), thread: ManuallyDrop::new(thread),
pollfd, pollfd,
display, display,
pid: Pid::from_raw(pid as _).expect("Xwayland PID was invalid?"),
} }
} }
fn new() -> Self { fn new() -> Self {
@ -262,13 +271,8 @@ impl Fixture {
&[connection.atoms.wm_delete_window], &[connection.atoms.wm_delete_window],
); );
self.testwl.close_toplevel(surface); self.testwl.close_toplevel(surface);
connection.await_event();
let event = connection
.inner
.poll_for_event()
.unwrap()
.expect("No close event");
let event = connection.await_event();
let xcb::Event::X(x::Event::ClientMessage(event)) = event else { let xcb::Event::X(x::Event::ClientMessage(event)) = event else {
panic!("Expected ClientMessage event, got {event:?}"); panic!("Expected ClientMessage event, got {event:?}");
}; };
@ -426,12 +430,19 @@ impl Connection {
} }
#[track_caller] #[track_caller]
fn await_event(&mut self) { #[must_use]
self.pollfd.clear_revents(); fn await_event(&mut self) -> xcb::Event {
if let Some(event) = self.poll_for_event().expect("Failed to poll for event") {
return event;
}
assert!( assert!(
poll(&mut [self.pollfd.clone()], 100).expect("poll failed") > 0, poll(&mut [self.pollfd.clone()], 100).expect("poll failed") > 0,
"Did not get any X11 events" "Did not get any X11 events"
); );
self.pollfd.clear_revents();
self.poll_for_event()
.expect("Failed to poll for event after pollfd")
.unwrap()
} }
#[track_caller] #[track_caller]
@ -462,18 +473,16 @@ impl Connection {
#[track_caller] #[track_caller]
fn await_selection_request(&mut self) -> x::SelectionRequestEvent { fn await_selection_request(&mut self) -> x::SelectionRequestEvent {
self.await_event(); match self.await_event() {
match self.poll_for_event().unwrap() { xcb::Event::X(x::Event::SelectionRequest(r)) => r,
Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r,
other => panic!("Didn't get selection request event, instead got {other:?}"), other => panic!("Didn't get selection request event, instead got {other:?}"),
} }
} }
#[track_caller] #[track_caller]
fn await_selection_notify(&mut self) -> x::SelectionNotifyEvent { fn await_selection_notify(&mut self) -> x::SelectionNotifyEvent {
self.await_event(); match self.await_event() {
match self.poll_for_event().unwrap() { xcb::Event::X(x::Event::SelectionNotify(r)) => r,
Some(xcb::Event::X(x::Event::SelectionNotify(r))) => r,
other => panic!("Didn't get selection notify event, instead got {other:?}"), other => panic!("Didn't get selection notify event, instead got {other:?}"),
} }
} }
@ -505,9 +514,8 @@ impl Connection {
#[track_caller] #[track_caller]
fn await_selection_owner_change(&mut self) -> xcb::xfixes::SelectionNotifyEvent { fn await_selection_owner_change(&mut self) -> xcb::xfixes::SelectionNotifyEvent {
self.await_event(); match self.await_event() {
match self.poll_for_event().unwrap() { xcb::Event::XFixes(xcb::xfixes::Event::SelectionNotify(e)) => e,
Some(xcb::Event::XFixes(xcb::xfixes::Event::SelectionNotify(e))) => e,
other => panic!("Expected XFixes SelectionNotify, got {other:?}"), other => panic!("Expected XFixes SelectionNotify, got {other:?}"),
} }
} }
@ -1131,7 +1139,6 @@ fn close_window() {
}) })
.unwrap(); .unwrap();
f.testwl.close_toplevel(surface); f.testwl.close_toplevel(surface);
connection.await_event();
f.wait_and_dispatch(); f.wait_and_dispatch();
// Connection should no longer work (KillClient) // Connection should no longer work (KillClient)
@ -1261,9 +1268,8 @@ fn incr_copy_from_x11() {
} }
assert_ne!(destination_property, x::Atom::none()); assert_ne!(destination_property, x::Atom::none());
connection.await_event(); let notify = match connection.await_event() {
let notify = match connection.poll_for_event().unwrap() { xcb::Event::X(x::Event::PropertyNotify(p)) => p,
Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
other => panic!("Didn't get property notify event, instead got {other:?}"), other => panic!("Didn't get property notify event, instead got {other:?}"),
}; };
@ -1337,7 +1343,6 @@ fn wayland_then_x11_clipboard_owner() {
f.testwl.dispatch(); f.testwl.dispatch();
connection.verify_clipboard_owner(window); connection.verify_clipboard_owner(window);
connection.await_event();
let request = connection.await_selection_request(); let request = connection.await_selection_request();
assert_eq!(request.selection(), connection.atoms.clipboard); assert_eq!(request.selection(), connection.atoms.clipboard);
assert_eq!(request.target(), connection.atoms.targets); assert_eq!(request.target(), connection.atoms.targets);