Skip to content

ovmm_entry: Remove need for unsafe in tests#3200

Merged
smalis-msft merged 3 commits intomicrosoft:mainfrom
smalis-msft:no-cli-unsafe
Apr 7, 2026
Merged

ovmm_entry: Remove need for unsafe in tests#3200
smalis-msft merged 3 commits intomicrosoft:mainfrom
smalis-msft:no-cli-unsafe

Conversation

@smalis-msft
Copy link
Copy Markdown
Contributor

This file really shouldn't need unsafe, we can just mock out the environment variable instead of actually modifying it.

@smalis-msft smalis-msft requested a review from a team as a code owner April 6, 2026 16:14
Copilot AI review requested due to automatic review settings April 6, 2026 16:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the need for unsafe in openvmm_entry CLI parsing tests by refactoring the autocache disk-spec parsing so tests can inject a cache-path value instead of mutating process-wide environment variables.

Changes:

  • Added DiskCliKind::parse_autocache(...) helper to parse autocache:[key]:<kind> given a cache path input.
  • Updated DiskCliKind’s FromStr implementation to use the new helper when parsing autocache.
  • Reworked tests to avoid std::env::set_var/remove_var and added coverage for key/no-key and missing-cache-path cases.

Copilot AI review requested due to automatic review settings April 7, 2026 14:46
@smalis-msft smalis-msft enabled auto-merge (squash) April 7, 2026 14:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@smalis-msft smalis-msft merged commit 46db2be into microsoft:main Apr 7, 2026
65 checks passed
@smalis-msft smalis-msft deleted the no-cli-unsafe branch April 7, 2026 20:13
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.

3 participants