Skip to content

Commit c06b308

Browse files
authored
test: add firmware service unit tests (#85)
## Summary Expand firmware service test coverage to validate file cleanup operations and device service client delegation. Tests now support parallel execution using mutex lock pattern (refactored from initial implementation). ## Tests Added (6 new, 7 total) ### clear_data_folder (3 tests) - ✅ Removes all files in data directory - ✅ Succeeds with empty directory - ✅ Preserves subdirectories (only files removed) ### load_update (2 tests) - ✅ Forwards request to device service with correct path - ✅ Returns error on device service failure ### run_update (2 tests) - ✅ Forwards request to device service - ✅ Returns error on device service failure ## Implementation Details - Uses `mockall` to mock `DeviceServiceClient` for async operations - Tests validate delegation logic without testing device service internals - Uses `serde_json` to construct `RunUpdate` (private fields) - Tests ensure directory exists before operations - **Parallel test support:** Uses static `DATA_FOLDER_LOCK` mutex following the `PASSWORD_FILE_LOCK` pattern for test isolation ## Refactoring for Parallel Execution Initially, `clear_data_folder` tests had a race condition due to shared temp directory from `AppConfig`. This has been resolved by: 1. Adding static `DATA_FOLDER_LOCK` mutex (following password.rs pattern) 2. Adding `lock_for_test()` method to `FirmwareService` 3. Updating tests to acquire lock before accessing shared data directory 4. Tests now run safely in parallel without `--test-threads=1` flag ## Known Limitations **File Upload Not Tested:** `handle_uploaded_firmware` is not tested as it requires mocking `actix_multipart::form::tempfile::TempFile`, which is complex. ## Test Results ```bash # All tests pass in parallel cargo test --features mock firmware::tests ``` Signed-off-by: Jan Zachmann <[email protected]>
1 parent dfaae6c commit c06b308

File tree

1 file changed

+188
-24
lines changed

1 file changed

+188
-24
lines changed

src/backend/src/services/firmware.rs

Lines changed: 188 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,26 @@ use anyhow::{Context, Result};
88
use log::{debug, error};
99
use std::{fs, os::unix::fs::PermissionsExt};
1010

11+
#[cfg(any(test, feature = "mock"))]
12+
use std::sync::{LazyLock, Mutex, MutexGuard};
13+
14+
#[cfg(any(test, feature = "mock"))]
15+
#[allow(dead_code)]
16+
static DATA_FOLDER_LOCK: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
17+
1118
/// Service for firmware update file operations
1219
pub struct FirmwareService;
1320

1421
impl FirmwareService {
22+
/// Acquire a lock for data folder operations (test-only)
23+
///
24+
/// This ensures that tests modifying the data folder don't interfere with each other
25+
#[cfg(any(test, feature = "mock"))]
26+
#[allow(dead_code)]
27+
pub fn lock_for_test() -> MutexGuard<'static, ()> {
28+
DATA_FOLDER_LOCK.lock().unwrap()
29+
}
30+
1531
/// Handle uploaded firmware file - clears data folder and persists the file
1632
///
1733
/// # Arguments
@@ -103,29 +119,177 @@ mod tests {
103119
use std::fs::File;
104120
use std::io::Write as _;
105121

106-
#[test]
107-
fn test_clear_data_folder() {
108-
let data_path = &AppConfig::get().paths.data_dir;
109-
110-
// Create some test files
111-
File::create(data_path.join("file1.txt"))
112-
.expect("should create file1")
113-
.write_all(b"test")
114-
.expect("should write");
115-
File::create(data_path.join("file2.txt"))
116-
.expect("should create file2")
117-
.write_all(b"test")
118-
.expect("should write");
119-
120-
// Verify files exist
121-
assert!(data_path.join("file1.txt").exists());
122-
assert!(data_path.join("file2.txt").exists());
123-
124-
// Clear folder (testing private method via module visibility)
125-
FirmwareService::clear_data_folder().expect("should clear folder");
126-
127-
// Verify files are deleted
128-
assert!(!data_path.join("file1.txt").exists());
129-
assert!(!data_path.join("file2.txt").exists());
122+
#[cfg(feature = "mock")]
123+
use mockall_double::double;
124+
125+
#[cfg(feature = "mock")]
126+
#[double]
127+
use crate::omnect_device_service_client::DeviceServiceClient;
128+
129+
mod clear_data_folder {
130+
use super::*;
131+
132+
#[test]
133+
fn removes_all_files() {
134+
let _lock = FirmwareService::lock_for_test();
135+
let data_path = &AppConfig::get().paths.data_dir;
136+
137+
// Ensure directory exists
138+
fs::create_dir_all(data_path).expect("should create data dir");
139+
140+
// Create some test files
141+
File::create(data_path.join("file1.txt"))
142+
.expect("should create file1")
143+
.write_all(b"test")
144+
.expect("should write");
145+
File::create(data_path.join("file2.txt"))
146+
.expect("should create file2")
147+
.write_all(b"test")
148+
.expect("should write");
149+
150+
// Verify files exist
151+
assert!(data_path.join("file1.txt").exists());
152+
assert!(data_path.join("file2.txt").exists());
153+
154+
// Clear folder
155+
FirmwareService::clear_data_folder().expect("should clear folder");
156+
157+
// Verify files are deleted
158+
assert!(!data_path.join("file1.txt").exists());
159+
assert!(!data_path.join("file2.txt").exists());
160+
}
161+
162+
#[test]
163+
fn succeeds_with_empty_directory() {
164+
let _lock = FirmwareService::lock_for_test();
165+
let data_path = &AppConfig::get().paths.data_dir;
166+
167+
// Ensure directory exists and is empty
168+
fs::create_dir_all(data_path).expect("should create data dir");
169+
170+
// Clear folder when already empty
171+
let result = FirmwareService::clear_data_folder();
172+
assert!(result.is_ok());
173+
}
174+
175+
#[test]
176+
fn preserves_subdirectories() {
177+
let _lock = FirmwareService::lock_for_test();
178+
let data_path = &AppConfig::get().paths.data_dir;
179+
180+
// Ensure directory exists
181+
fs::create_dir_all(data_path).expect("should create data dir");
182+
183+
// Create a subdirectory
184+
let subdir = data_path.join("subdir");
185+
fs::create_dir_all(&subdir).expect("should create subdir");
186+
187+
// Create a file in root
188+
File::create(data_path.join("file.txt"))
189+
.expect("should create file")
190+
.write_all(b"test")
191+
.expect("should write");
192+
193+
// Clear folder
194+
FirmwareService::clear_data_folder().expect("should clear folder");
195+
196+
// File should be deleted
197+
assert!(!data_path.join("file.txt").exists());
198+
199+
// Subdirectory should still exist (only files are removed)
200+
assert!(subdir.exists());
201+
assert!(subdir.is_dir());
202+
203+
// Cleanup
204+
let _ = fs::remove_dir(&subdir);
205+
}
206+
}
207+
208+
mod load_update {
209+
use super::*;
210+
use crate::omnect_device_service_client::LoadUpdate;
211+
212+
#[tokio::test]
213+
async fn forwards_request_to_device_service() {
214+
let mut device_mock = DeviceServiceClient::default();
215+
216+
device_mock
217+
.expect_load_update()
218+
.withf(|req: &LoadUpdate| {
219+
req.update_file_path == AppConfig::get().paths.host_update_file
220+
})
221+
.times(1)
222+
.returning(|_| Box::pin(async { Ok("update loaded successfully".to_string()) }));
223+
224+
let result = FirmwareService::load_update(&device_mock).await;
225+
226+
assert!(result.is_ok());
227+
assert_eq!(result.unwrap(), "update loaded successfully");
228+
}
229+
230+
#[tokio::test]
231+
async fn returns_error_on_device_service_failure() {
232+
let mut device_mock = DeviceServiceClient::default();
233+
234+
device_mock
235+
.expect_load_update()
236+
.returning(|_| Box::pin(async { Err(anyhow::anyhow!("device service error")) }));
237+
238+
let result = FirmwareService::load_update(&device_mock).await;
239+
240+
assert!(result.is_err());
241+
assert!(
242+
result
243+
.unwrap_err()
244+
.to_string()
245+
.contains("device service error")
246+
);
247+
}
248+
}
249+
250+
mod run_update {
251+
use super::*;
252+
253+
#[tokio::test]
254+
async fn forwards_request_to_device_service() {
255+
let mut device_mock = DeviceServiceClient::default();
256+
257+
device_mock
258+
.expect_run_update()
259+
.times(1)
260+
.returning(|_| Box::pin(async { Ok(()) }));
261+
262+
// Create RunUpdate via serde (since fields are private)
263+
let run_update: crate::omnect_device_service_client::RunUpdate =
264+
serde_json::from_str(r#"{"validate_iothub_connection": true}"#)
265+
.expect("should deserialize");
266+
267+
let result = FirmwareService::run_update(&device_mock, run_update).await;
268+
269+
assert!(result.is_ok());
270+
}
271+
272+
#[tokio::test]
273+
async fn returns_error_on_device_service_failure() {
274+
let mut device_mock = DeviceServiceClient::default();
275+
276+
device_mock
277+
.expect_run_update()
278+
.returning(|_| Box::pin(async { Err(anyhow::anyhow!("update execution failed")) }));
279+
280+
let run_update: crate::omnect_device_service_client::RunUpdate =
281+
serde_json::from_str(r#"{"validate_iothub_connection": false}"#)
282+
.expect("should deserialize");
283+
284+
let result = FirmwareService::run_update(&device_mock, run_update).await;
285+
286+
assert!(result.is_err());
287+
assert!(
288+
result
289+
.unwrap_err()
290+
.to_string()
291+
.contains("update execution failed")
292+
);
293+
}
130294
}
131295
}

0 commit comments

Comments
 (0)