Re: [PATCH v5 proxmox-backup 03/16] datastore: add TempTestDir that is automatically deleted on drop

["Lukas Wagner" <l.wagner@proxmox.com>]

On Mon Feb 2, 2026 at 11:12 AM CET, Robert Obkircher wrote:
>
> On 2/2/26 09:30, Lukas Wagner wrote:
>> On Fri Jan 30, 2026 at 5:45 PM CET, Robert Obkircher wrote:
>>> This will be used to test the FixedIndexWriter and chunk store
>>> creation.
>>>
>>> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
>>> ---
>>>  pbs-datastore/src/lib.rs           |  4 ++++
>>>  pbs-datastore/src/temp_test_dir.rs | 33 ++++++++++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>  create mode 100644 pbs-datastore/src/temp_test_dir.rs
>>>
>>> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs
>>> index 1f7c54ae..b48ec93c 100644
>>> --- a/pbs-datastore/src/lib.rs
>>> +++ b/pbs-datastore/src/lib.rs
>>> @@ -233,3 +233,7 @@ pub use local_chunk_reader::LocalChunkReader;
>>>  
>>>  mod local_datastore_lru_cache;
>>>  pub use local_datastore_lru_cache::LocalDatastoreLruCache;
>>> +
>>> +#[cfg(test)]
>>> +mod temp_test_dir;
>>> +
>>> diff --git a/pbs-datastore/src/temp_test_dir.rs b/pbs-datastore/src/temp_test_dir.rs
>>> new file mode 100644
>>> index 00000000..13e16c91
>>> --- /dev/null
>>> +++ b/pbs-datastore/src/temp_test_dir.rs
>>> @@ -0,0 +1,33 @@
>>> +use std::convert::From;
>>> +use std::env;
>>> +use std::path::{Path, PathBuf};
>>> +
>>> +/// A temporary directory for tests that is removed on drop.
>>> +/// Deletion is best-effort and errors are silently ignored.
>>> +pub struct TempTestDir(PathBuf);
>>> +
>>> +impl TempTestDir {
>>> +    /// Create an empty directory.
>>> +    pub fn new() -> Self {
>>> +        Self(proxmox_sys::fs::make_tmp_dir(env::temp_dir(), None).unwrap())
>>> +    }
>>> +}
>>> +
>>> +impl std::ops::Deref for TempTestDir {
>>> +    type Target = Path;
>>> +    fn deref(&self) -> &Self::Target {
>>> +        &self.0
>>> +    }
>>> +}
>>> +
>>> +impl Drop for TempTestDir {
>>> +    fn drop(&mut self) {
>>> +        let _ = std::fs::remove_dir_all(&self.0);
>>> +    }
>>> +}
>>> +
>>> +impl<'a> From<&'a TempTestDir> for PathBuf {
>>> +    fn from(temp: &'a TempTestDir) -> Self {
>>> +        temp.0.clone()
>>> +    }
>>> +}
>>
>> Hey Robert,
>>
>> We do already have a very similar helper in the PDM repository [1].
>> Maybe it would sense move either this or the PDM version (or some hybrid)
>> into a shared crate in proxmox.git?
>>
>> I would suggest creating a new crate for commonly used test-only
>> helpers (proxmox-test-support, proxmox-test-helpers, proxmox-test-util,
>> maybe you have a better suggestion?)
>>
>> What do you think?
> I've noticed this as well when I skimmed your RFC.
>
> I'm not sure if combining unrelated things into a -test
> crate is a good idea. Maybe a (proxmox-)tempfile crate
> that is added to [dev-dependencies] would be slightly
> nicer?

Also okay to me. Then I would make sure that this helper is
'production-ready' - so maybe log the error in case `drop` could not
clean up the directory, add support for `CreateOptions`, and also maybe
add a method that allows one to clean up manually, so that any potential
error can be handle by the application if needed (TmpDir from the
tempfile crate has a `close` method, something like that).

Maybe it could also make sense to move the tmpfile helpers from
proxmox-sys there, if possible (IIRC proxmox-sys uses the tempfile
helpers internally).

>
> I've also considered adding it to proxmox_sys::fs::dir,
> but it does seem a little out of place there.

Yeah, I wouldn't do that. proxmox-sys is pretty huge already and should
rather be broken apart into meaningful smaller crates than made bigger.

>
>>
>> [1] https://git.proxmox.com/?p=proxmox-datacenter-manager.git;a=blob;f=server/src/test_support/temp.rs;h=a93c914d9efcc60de3bb40bc79610abc98571b7f;hb=HEAD