Re: [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type

[Robert Obkircher <r.obkircher@proxmox.com>]

> Unfortunately, some subsystems still require static initialization,
> namely access control and worker tasks. For those we need a setup
> function that is ensure to be only called once in each test binary.
> 
> Worker tasks especially require a directory in which it can place task
> logs. For this we create a temporary directory, which we clean up using
> libc::atexit. Unfortunately, tempfile::TempDir does not work for this
> case, since we'd need to put it in a static variable, and `drop` is
> never called for those.
> 
> TestApplication is essentially a builder that can be used to produce a
> PdmApplication suitable for tests.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>
> diff --git a/server/tests/common/environment.rs b/server/tests/common/environment.rs
> new file mode 100644
> index 00000000..5dad7b21
> --- /dev/null
> +++ b/server/tests/common/environment.rs
> @@ -0,0 +1,25 @@
> +use proxmox_router::RpcEnvironment;
> +
> +pub struct TestRpcEnvironment;
> +
> +impl RpcEnvironment for TestRpcEnvironment {
> +    fn result_attrib_mut(&mut self) -> &mut serde_json::Value {
> +        unimplemented!()
> +    }
> +
> +    fn result_attrib(&self) -> &serde_json::Value {
> +        unimplemented!()
> +    }
> +
> +    fn env_type(&self) -> proxmox_router::RpcEnvironmentType {
> +        unimplemented!()
> +    }
> +
> +    fn set_auth_id(&mut self, _user: Option<String>) {
> +        unimplemented!()
> +    }
> +
> +    fn get_auth_id(&self) -> Option<String> {
> +        Some("root@pam".to_string())
> +    }
> +}
> diff --git a/server/tests/common/mod.rs b/server/tests/common/mod.rs
> new file mode 100644
> index 00000000..49646706
> --- /dev/null
> +++ b/server/tests/common/mod.rs
> @@ -0,0 +1,56 @@
> +use std::{
> +    path::{Path, PathBuf},
> +    sync::{Once, OnceLock},
> +};
> +
> +use anyhow::Context;
> +use serde::de::DeserializeOwned;
> +
> +use proxmox_sys::fs::CreateOptions;
> +
> +mod environment;
> +mod test_application;
> +
> +pub use environment::TestRpcEnvironment;
> +pub use test_application::*;
> +
> +pub async fn read_captured_response<T: DeserializeOwned, P: AsRef<Path>>(
> +    path: P,
> +) -> Result<T, proxmox_client::Error> {
> +    let s = tokio::fs::read_to_string(path.as_ref())
> +        .await
> +        .with_context(|| format!("could not read from {path}", path = path.as_ref().display()))
> +        .unwrap();
> +    Ok(serde_json::from_str(&s).unwrap())
> +}
> +
> +static STATIC_TEMP_DIR: OnceLock<PathBuf> = OnceLock::new();
> +
> +extern "C" fn cleanup() {
> +    if let Some(dir) = STATIC_TEMP_DIR.get() {
> +        let _ = std::fs::remove_dir_all(dir);
> +    }
> +}
> +
> +pub fn test_setup() {
> +    static INIT: Once = Once::new();
> +
> +    INIT.call_once(|| {
> +        let file_opts = CreateOptions::new();
> +
> +        let dir = proxmox_sys::fs::make_tmp_dir("/tmp", None).unwrap();

A while ago I had a brief discussion with Fabian about how file I/O
should be tested. He argued that tests should not make any assumptions
about the file system outside of CARGO_TARGET_TMPDIR, not even about
/tmp.

But that is only available for real integration tests, not for
unittests.

> +        STATIC_TEMP_DIR.set(dir.clone()).unwrap();
> +
> +        proxmox_rest_server::init_worker_tasks(dir.clone(), file_opts).unwrap();
> +        proxmox_access_control::init::init(&pdm_api_types::AccessControlConfig, dir)
> +            .expect("failed to setup access control config");
> +
> +        unsafe {
> +            libc::atexit(cleanup);

Doesn't this potentially delete files while other threads are stil
accessing them?

-- 
Robert Obkircher <r.obkircher@proxmox.com>