Merge pull request #718 from imobachgs/tests

Integration tests for `agama-dbus-server`

rust/agama-dbus-server/src/main.rs

@@ -1,4 +1,4 @@
-use agama_dbus_server::{locale, network::NetworkService, questions};
+use agama_dbus_server::{locale, network, questions};
 
 use log::LevelFilter;
 use std::future::pending;
@@ -28,7 +28,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     log::info!("Started questions interface");
     let _conn = locale::start_service(ADDRESS).await?;
     log::info!("Started locale interface");
-    NetworkService::start(ADDRESS).await?;
+    network::start_service(ADDRESS).await?;
     log::info!("Started network interface");
 
     // Do other things or go to wait forever

rust/agama-dbus-server/src/network.rs

@@ -50,4 +50,12 @@ pub use action::Action;
 pub use adapter::Adapter;
 pub use dbus::NetworkService;
 pub use model::NetworkState;
+pub use nm::NetworkManagerAdapter;
 pub use system::NetworkSystem;
+
+pub async fn start_service(address: &str) -> Result<(), Box<dyn std::error::Error>> {
+    let adapter = NetworkManagerAdapter::from_system()
+        .await
+        .expect("Could not connect to NetworkManager to read the configuration.");
+    NetworkService::start(address, adapter).await
+}

rust/agama-dbus-server/src/network/dbus/interfaces.rs

@@ -19,6 +19,7 @@ use std::net::{AddrParseError, Ipv4Addr};
 use zbus::{
     dbus_interface,
     zvariant::{ObjectPath, OwnedObjectPath},
+    SignalContext,
 };
 
 /// D-Bus interface for the network devices collection
@@ -126,7 +127,7 @@ impl Connections {
     pub async fn add_connection(&mut self, id: String, ty: u8) -> zbus::fdo::Result<()> {
         let actions = self.actions.lock().await;
         actions
-            .send(Action::AddConnection(id, ty.try_into()?))
+            .send(Action::AddConnection(id.clone(), ty.try_into()?))
             .await
             .unwrap();
         Ok(())
@@ -163,6 +164,13 @@ impl Connections {
         actions.send(Action::Apply).await.unwrap();
         Ok(())
     }
+
+    #[dbus_interface(signal)]
+    pub async fn connection_added(
+        ctxt: &SignalContext<'_>,
+        id: &str,
+        path: &str,
+    ) -> zbus::Result<()>;
 }
 
 /// D-Bus interface for a network connection

rust/agama-dbus-server/src/network/dbus/service.rs

@@ -1,7 +1,7 @@
 //! Network D-Bus service.
 //!
 //! This module defines a D-Bus service which exposes Agama's network configuration.
-use crate::network::NetworkSystem;
+use crate::network::{Adapter, NetworkSystem};
 use agama_lib::connection_to;
 use std::error::Error;
 
@@ -12,13 +12,14 @@ pub struct NetworkService;
 
 impl NetworkService {
     /// Starts listening and dispatching events on the D-Bus connection.
-    pub async fn start(address: &str) -> Result<(), Box<dyn Error>> {
+    pub async fn start<T: Adapter + std::marker::Send + 'static>(
+        address: &str,
+        adapter: T,
+    ) -> Result<(), Box<dyn Error>> {
         const SERVICE_NAME: &str = "org.opensuse.Agama.Network1";
 
         let connection = connection_to(address).await?;
-        let mut network = NetworkSystem::from_network_manager(connection.clone())
-            .await
-            .expect("Could not read network state");
+        let mut network = NetworkSystem::new(connection.clone(), adapter);
 
         async_std::task::spawn(async move {
             network

rust/agama-dbus-server/src/network/dbus/tree.rs

@@ -79,9 +79,15 @@ impl Tree {
     /// Adds a connection to the D-Bus tree.
     ///
     /// * `connection`: connection to add.
-    pub async fn add_connection(&self, conn: &mut Connection) -> Result<(), ServiceError> {
+    /// * `notify`: whether to notify the added connection
+    pub async fn add_connection(
+        &self,
+        conn: &mut Connection,
+        notify: bool,
+    ) -> Result<(), ServiceError> {
         let mut objects = self.objects.lock().await;
 
+        let orig_id = conn.id().to_owned();
         let (id, path) = objects.register_connection(conn);
         if id != conn.id() {
             conn.set_id(&id)
@@ -106,6 +112,10 @@ impl Tree {
             .await?;
         }
 
+        if notify {
+            self.notify_connection_added(&orig_id, &path).await?;
+        }
+
         Ok(())
     }
 
@@ -115,7 +125,7 @@ impl Tree {
     pub async fn remove_connection(&mut self, id: &str) -> Result<(), ServiceError> {
         let mut objects = self.objects.lock().await;
         let Some(path) = objects.connection_path(id) else {
-            return Ok(())
+            return Ok(());
         };
         self.remove_connection_on(path.as_str()).await?;
         objects.deregister_connection(id).unwrap();
@@ -127,7 +137,7 @@ impl Tree {
     /// * `connections`: list of connections.
     async fn add_connections(&self, connections: &mut [Connection]) -> Result<(), ServiceError> {
         for conn in connections.iter_mut() {
-            self.add_connection(conn).await?;
+            self.add_connection(conn, false).await?;
         }
 
         self.add_interface(
@@ -182,6 +192,15 @@ impl Tree {
         let object_server = self.connection.object_server();
         Ok(object_server.at(path, iface).await?)
     }
+
+    /// Notify that a new connection has been added
+    async fn notify_connection_added(&self, id: &str, path: &str) -> Result<(), ServiceError> {
+        let object_server = self.connection.object_server();
+        let iface_ref = object_server
+            .interface::<_, interfaces::Connections>(CONNECTIONS_PATH)
+            .await?;
+        Ok(interfaces::Connections::connection_added(iface_ref.signal_context(), id, path).await?)
+    }
 }
 
 /// Objects paths for known devices and connections

rust/agama-dbus-server/src/network/model.rs

@@ -13,7 +13,7 @@ use std::{
 };
 use thiserror::Error;
 
-#[derive(Default)]
+#[derive(Default, Clone)]
 pub struct NetworkState {
     pub devices: Vec<Device>,
     pub connections: Vec<Connection>,

rust/agama-dbus-server/src/network/system.rs

@@ -1,53 +1,36 @@
-use crate::network::{
-    dbus::Tree, model::Connection, nm::NetworkManagerAdapter, Action, Adapter, NetworkState,
-};
-use agama_lib::error::ServiceError;
+use crate::network::{dbus::Tree, model::Connection, Action, Adapter, NetworkState};
 use async_std::channel::{unbounded, Receiver, Sender};
 use std::error::Error;
 
-/// Represents the network system, wrapping a [NetworkState] and setting up the D-Bus tree.
-pub struct NetworkSystem {
+/// Represents the network system using holding the state and setting up the D-Bus tree.
+pub struct NetworkSystem<T: Adapter> {
     /// Network state
     pub state: NetworkState,
     /// Side of the channel to send actions.
     actions_tx: Sender<Action>,
     actions_rx: Receiver<Action>,
     tree: Tree,
+    /// Adapter to read/write the network state.
+    adapter: T,
 }
 
-impl NetworkSystem {
-    pub fn new(state: NetworkState, conn: zbus::Connection) -> Self {
+impl<T: Adapter> NetworkSystem<T> {
+    pub fn new(conn: zbus::Connection, adapter: T) -> Self {
         let (actions_tx, actions_rx) = unbounded();
         let tree = Tree::new(conn, actions_tx.clone());
         Self {
-            state,
+            state: NetworkState::default(),
             actions_tx,
             actions_rx,
             tree,
+            adapter,
         }
     }
 
-    /// Reads the network configuration using the NetworkManager adapter.
-    ///
-    /// * `conn`: connection where self will be exposed. Another connection will be made internally
-    ///   to talk with NetworkManager (which may be on a different bus even).
-    pub async fn from_network_manager(
-        conn: zbus::Connection,
-    ) -> Result<NetworkSystem, Box<dyn Error>> {
-        let adapter = NetworkManagerAdapter::from_system()
-            .await
-            .expect("Could not connect to NetworkManager to read the configuration.");
-        let state = adapter.read()?;
-        Ok(Self::new(state, conn))
-    }
-
-    /// Writes the network configuration to NetworkManager.
-    pub async fn to_network_manager(&mut self) -> Result<(), Box<dyn Error>> {
-        let adapter = NetworkManagerAdapter::from_system()
-            .await
-            .expect("Could not connect to NetworkManager to write the changes.");
-        adapter.write(&self.state)?;
-        self.state = adapter.read()?;
+    /// Writes the network configuration.
+    pub async fn write(&mut self) -> Result<(), Box<dyn Error>> {
+        self.adapter.write(&self.state)?;
+        self.state = self.adapter.read()?;
         Ok(())
     }
 
@@ -58,7 +41,8 @@ impl NetworkSystem {
     }
 
     /// Populates the D-Bus tree with the known devices and connections.
-    pub async fn setup(&mut self) -> Result<(), ServiceError> {
+    pub async fn setup(&mut self) -> Result<(), Box<dyn Error>> {
+        self.state = self.adapter.read()?;
         self.tree
             .set_connections(&mut self.state.connections)
             .await?;
@@ -82,7 +66,7 @@ impl NetworkSystem {
         match action {
             Action::AddConnection(name, ty) => {
                 let mut conn = Connection::new(name, ty);
-                self.tree.add_connection(&mut conn).await?;
+                self.tree.add_connection(&mut conn, true).await?;
                 self.state.add_connection(conn)?;
             }
             Action::UpdateConnection(conn) => {
@@ -93,7 +77,7 @@ impl NetworkSystem {
                 self.state.remove_connection(&id)?;
             }
             Action::Apply => {
-                self.to_network_manager().await?;
+                self.write().await?;
                 // TODO: re-creating the tree is kind of brute-force and it sends signals about
                 // adding/removing interfaces. We should add/update/delete objects as needed.
                 self.tree

rust/agama-dbus-server/tests/common/mod.rs

@@ -0,0 +1,104 @@
+use agama_lib::{connection_to, error::ServiceError};
+use async_std::stream::StreamExt;
+use std::process::{Child, Command};
+use std::time::Duration;
+use uuid::Uuid;
+use zbus::{MatchRule, MessageStream, MessageType};
+
+/// D-Bus server to be used on tests.
+///
+/// This struct takes care of starting, stopping and monitoring dbus-daemon to be used on
+/// integration tests. Each server uses a different socket, so they do not collide.
+pub struct DBusServer {
+    child: Option<Child>,
+    messages: Option<MessageStream>,
+    pub address: String,
+}
+
+impl DBusServer {
+    /// Builds and starts a server.
+    pub async fn start_server() -> Result<Self, ServiceError> {
+        let mut server = Self::new();
+        server.start().await?;
+        println!("Starting the server at {}", &server.address);
+        Ok(server)
+    }
+
+    /// Builds a new server.
+    ///
+    /// To start the server, check the `start` function.
+    pub fn new() -> Self {
+        let uuid = Uuid::new_v4();
+        Self {
+            address: format!("unix:path=/tmp/agama-tests-{uuid}"),
+            child: None,
+            messages: None,
+        }
+    }
+
+    /// Starts the server.
+    pub async fn start(&mut self) -> Result<(), ServiceError> {
+        let child = Command::new("/usr/bin/dbus-daemon")
+            .args([
+                "--config-file",
+                "../share/dbus-test.conf",
+                "--address",
+                &self.address,
+            ])
+            .spawn()
+            .expect("to start the testing D-Bus daemon");
+        self.child = Some(child);
+        self.wait();
+        self.messages = Some(self.build_message_stream().await?);
+        Ok(())
+    }
+
+    /// Stops the server.
+    pub fn stop(&mut self) {
+        if let Some(mut child) = self.child.take() {
+            child.kill().unwrap();
+        }
+        self.messages = None;
+    }
+
+    /// Waits for a server to be available.
+    ///
+    /// * `name`: service name (e.g., "org.opensuse.Agama.Network1").
+    pub async fn wait_for_service(&mut self, name: &str) {
+        let Some(ref mut messages) = self.messages else {
+            return;
+        };
+
+        loop {
+            let signal = messages.next().await.unwrap().unwrap();
+            let (sname, _, _): (String, String, String) = signal.body().unwrap();
+            if &sname == name {
+                return;
+            }
+        }
+    }
+
+    /// Waits until the D-Bus daemon is ready.
+    // TODO: implement proper waiting instead of just using a sleep
+    fn wait(&self) {
+        const WAIT_TIME: Duration = Duration::from_millis(500);
+        std::thread::sleep(WAIT_TIME);
+    }
+
+    /// Builds a message stream.
+    async fn build_message_stream(&self) -> Result<MessageStream, ServiceError> {
+        let conn = connection_to(&self.address).await?;
+        let rule = MatchRule::builder()
+            .msg_type(MessageType::Signal)
+            .sender("org.freedesktop.DBus")?
+            .member("NameOwnerChanged")?
+            .build();
+        Ok(MessageStream::for_match_rule(rule, &conn, None).await?)
+    }
+}
+
+impl Drop for DBusServer {
+    fn drop(&mut self) {
+        self.stop();
+    }
+}