Skip to content

Add shared test fixtures and helper functions - #76

Open
lforchini wants to merge 1 commit into
fix-double-path-bugfrom
test/test-utils
Open

lforchini wants to merge 1 commit into
fix-double-path-bugfrom
test/test-utils

Conversation

@lforchini

Copy link
Copy Markdown
Collaborator

Create a fixture which hooks into the custom-prefixed Path system, but
doesn't require an env var to control it. Instead, unit tests can create
a MockDir which will then act as the path prefix.

It should still allow for multiple tests to be run at the same time as
a thread_local is used to store the prefix.

Signed-off-by: Leonardo Forchini leonardo.forchini@nutanix.com


Stack created with GitHub Stacks CLI • Give Feedback 💬

@lforchini
lforchini added this pull request to stack #83 October 1, 2026 14:37
@lforchini
lforchini removed this pull request from stack #83 October 1, 2026 14:50
@lforchini
lforchini added this pull request to stack #84 October 1, 2026 14:50
@lforchini
lforchini removed this pull request from stack #84 October 5, 2026 08:12
@lforchini
lforchini added this pull request to stack #87 October 5, 2026 08:16
@lforchini
lforchini marked this pull request as ready for review October 5, 2026 08:26
@lforchini
lforchini removed this pull request from stack #87 October 8, 2026 15:22
@lforchini
lforchini added this pull request to stack #88 October 8, 2026 15:23
@lforchini
lforchini removed this pull request from stack #88 October 13, 2026 12:32
@lforchini
lforchini added this pull request to stack #95 October 13, 2026 12:44
@lforchini
lforchini removed this pull request from stack #152 October 23, 2026 08:44
@lforchini
lforchini changed the base branch from test/refactor-config-loading to fix-double-path-bug October 23, 2026 08:44
@lforchini
lforchini added this pull request to stack #157 October 23, 2026 08:45
@lforchini
lforchini removed this pull request from stack #157 October 23, 2026 08:49
@lforchini
lforchini added this pull request to stack #159 October 23, 2026 08:50
@lforchini
lforchini removed this pull request from stack #159 October 30, 2026 10:48
@lforchini
lforchini added this pull request to stack #173 October 30, 2026 10:50
@lforchini
lforchini removed this pull request from stack #173 November 5, 2026 15:02
@lforchini
lforchini added this pull request to stack #176 November 5, 2026 15:03
@lforchini
lforchini removed this pull request from stack #176 November 5, 2026 15:09
@lforchini
lforchini added this pull request to stack #178 November 5, 2026 15:10
@lforchini
lforchini removed this pull request from stack #178 November 6, 2026 10:23
@lforchini
lforchini added this pull request to stack #180 November 6, 2026 10:24
@lforchini
lforchini force-pushed the test/test-utils branch 2 times, most recently from c0d134f to 0e5f4de Compare November 13, 2026 09:38
Create a fixture which hooks into the custom-prefixed Path system, but
doesn't require an env var to control it. Instead, unit tests can create
a MockDir which will then act as the path prefix.

It should still allow for multiple tests to be run at the same time as
a thread_local is used to store the prefix.

Signed-off-by: Leonardo Forchini <leonardo.forchini@nutanix.com>

@tmakatos tmakatos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you reword the commit message to state first what the problem is?

Comment thread src/test/mockdir.rs
pub static ROOT_OVERRIDE: RefCell<Option<PathBuf>> = const { RefCell::new(None) };
}

/// Scoped root override for tests that access absolute paths through

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get what you mean by "scoped" here.

Comment thread src/test/mockdir.rs
//
// Author: Leonardo Forchini <leonardo.forchini@nutanix.com>
//
//! `tempfile::TempDir` wrapper which controls the prefix of `util::Path`s.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate? How does it control it? Why is it needed? What's the use case?

Comment thread src/test/mockdir.rs
}

/// Override the root used by `util::Path` on the current test thread.
pub(crate) fn override_root(root: impl Into<PathBuf>) -> RootOverride {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I presume this replaces the first absolute component with root?

Comment thread src/test/mockdir.rs
}

/// Override the root used by `util::Path` on the current test thread.
pub(crate) fn override_root(root: impl Into<PathBuf>) -> RootOverride {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why is this a free function and not in impl MockDir?

Comment thread src/controller.rs
engines::{EngineTickContext, ScalingEngine},
instance::{InstanceClient, ThreadPoolSnapshot},
per_vm_log::PerVmLog,
test::MockDir,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, test::{MockDir, mockdir::mockdir}

Comment thread src/path.rs
}

static IO_THREAD_CONTROLLER_ROOT_PATH: LazyLock<std::path::PathBuf> = LazyLock::new(|| {
static ROOT_PATH: LazyLock<std::path::PathBuf> = LazyLock::new(|| {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the rename?

Comment thread src/path.rs
)
});

/// Returns the path new `util::Path`s will have prefixed. In non-test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this sentence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants