public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Robert Obkircher" <r.obkircher@proxmox.com>,
	"Lukas Wagner" <l.wagner@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type
Date: Mon, 24 Aug 2026 15:13:54 +0200	[thread overview]
Message-ID: <DKX78SOV52S5.3NKQSHFXV5Z35@proxmox.com> (raw)
In-Reply-To: <178732083104.243770.17090706260253141150.b4-review@b4>

On Fri Aug 21, 2026 at 4:00 PM CEST, Robert Obkircher wrote:
>> +
>> +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.

Yeah, might make it sense to use it for the tests here, I guess.

Right now, we have quite a few unit tests in PDM which use /tmp anyways,
for exactly the reason you mentioned. Wondering if there is any sensible
approach for these.

>
>> +        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?

Its a bit of a crutch admittedly, but I think in the context of integration tests
this should not be a huge issue? If exit() is called, any remaining
thread is stopped anyway; it should not be a big deal if any file IO in
the temporary directory fails then?

Admittedly, I haven't examined the exact behavior of rust's test runner,
so maybe I'm missing something here.





  reply	other threads:[~2026-08-24 13:13 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 12:57 [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 01/20] router: introduce shared state Lukas Wagner
2026-08-17 13:26   ` Lukas Wagner
2026-08-20 11:23   ` Lukas Wagner
2026-08-21 13:59   ` Robert Obkircher
2026-08-24 13:13     ` Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 02/20] rest-server: allow to inject " Lukas Wagner
2026-08-17 12:57 ` [PATCH proxmox 03/20] api-macro: support shared state extraction type Lukas Wagner
2026-08-21 13:59   ` Robert Obkircher
2026-08-24 13:13     ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 04/20] context: promote context to a dir-style module Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 05/20] pdm-config: remotes: rename trait methods to read/write/lock Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 06/20] pdm-config: subscriptions: " Lukas Wagner
2026-08-21 14:00   ` Robert Obkircher
2026-08-24 13:13     ` Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 07/20] remote iterator: pass remote config reader explicitly Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 08/20] context: introduce a ContextFactory to build application context Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 09/20] context: establish PdmApplication object Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 10/20] context: register PdmApplication in router Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 11/20] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
2026-08-21 14:00   ` Robert Obkircher
2026-08-24 13:13     ` Lukas Wagner [this message]
2026-08-17 12:57 ` [PATCH datacenter-manager 12/20] parallel fetcher: support a custom client factory Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 13/20] api: sdn: use PdmApplication handle for accessing remotes Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 14/20] tests: add helpers for building API-handler-level integration tests Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 15/20] tests: add example tests for SDN API routes Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 16/20] api-cache: add wrapper type Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 17/20] context: provide api-cache on the app object Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 18/20] api: subscriptions: use PdmApplication instead of globals Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 19/20] pdm-config: subscriptions: drop unused accessor functions Lukas Wagner
2026-08-17 12:57 ` [PATCH datacenter-manager 20/20] tests: add example tests for remote subscription management Lukas Wagner
2026-08-20 14:54 ` superseded: [PATCH datacenter-manager/proxmox 00/20] inject application context via API macro for easier integration testing Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKX78SOV52S5.3NKQSHFXV5Z35@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=r.obkircher@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal