Skip to content

Conversation

@cody-herbst
Copy link

@cody-herbst cody-herbst commented Oct 30, 2025

Description

part of #361

Implemented the linux_ns_path integration test

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test updates
  • CI/CD related changes
  • Other (please describe):

Testing

  • Added new unit tests
  • Added new integration tests
  • Ran existing test suite
  • Tested manually (please provide steps)

Related Issues

Fixes #

Additional Context

@cody-herbst cody-herbst force-pushed the linux_ns_path_contest_integration_test branch from ccdec4f to d41b628 Compare October 30, 2025 16:39
@utam0k utam0k requested review from a team and Copilot November 1, 2025 12:43
Copy link
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

This PR implements integration tests for linux namespace path functionality as part of issue #361. The tests verify that containers can correctly inherit namespaces by comparing inodes between unshared processes and container processes.

  • Adds comprehensive integration tests for various Linux namespace types (PID, UTS, IPC, Mount, Network)
  • Implements a namespace path testing framework with proper cleanup mechanisms
  • Integrates the new test module into the existing test suite structure

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
tests/contest/contest/src/tests/mod.rs Adds module declaration for the new linux_ns_path test module
tests/contest/contest/src/tests/linux_ns_path/ns_path_test.rs Implements the core namespace path testing logic with cleanup utilities and test cases
tests/contest/contest/src/tests/linux_ns_path/mod.rs Module file exposing the test functionality
tests/contest/contest/src/main.rs Integrates the new test group into the main test runner

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cody-herbst cody-herbst force-pushed the linux_ns_path_contest_integration_test branch from 2d9833a to 547760e Compare November 2, 2025 18:44
@cody-herbst
Copy link
Author

Apologies for all the fix commits. I just notice the "just lint" command

@cody-herbst cody-herbst force-pushed the linux_ns_path_contest_integration_test branch from ba8180b to fdd3b27 Compare November 7, 2025 14:53
@cody-herbst cody-herbst requested a review from utam0k November 12, 2025 06:06
@utam0k utam0k requested a review from Copilot November 25, 2025 11:22
Copy link
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 4 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
}

fn create_spec(namespace_path: &Vec<NamespacePath>) -> Spec {
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The parameter should be a slice &[NamespacePath] instead of a reference to a Vec. This is more idiomatic Rust and allows the function to accept both Vec and array references.

Suggested change
fn create_spec(namespace_path: &Vec<NamespacePath>) -> Spec {
fn create_spec(namespace_path: &[NamespacePath]) -> Spec {

Copilot uses AI. Check for mistakes.
let child = match child {
Ok(child) => child,
Err(e) => {
return TestResult::Failed(anyhow!(format!("could not spawn unshare: {}", e)));
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The format!() call is unnecessary here. Use anyhow!() directly with format string arguments: anyhow!('could not spawn unshare: {}', e).

Suggested change
return TestResult::Failed(anyhow!(format!("could not spawn unshare: {}", e)));
return TestResult::Failed(anyhow!("could not spawn unshare: {}", e));

Copilot uses AI. Check for mistakes.
Comment on lines 171 to 174
return TestResult::Failed(anyhow!(format!(
"could not wait for path {}",
&namespace_path.path.display()
)));
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The format!() call is unnecessary here. Use anyhow!() directly with format string arguments: anyhow!('could not wait for path {}', namespace_path.path.display()).

Suggested change
return TestResult::Failed(anyhow!(format!(
"could not wait for path {}",
&namespace_path.path.display()
)));
return TestResult::Failed(anyhow!(
"could not wait for path {}",
&namespace_path.path.display()
));

Copilot uses AI. Check for mistakes.
Comment on lines 191 to 195
return TestResult::Failed(anyhow!(format!(
"could not get inode of {}: {}",
&unshared_namespace_path.path.display(),
e
)));
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The format!() call is unnecessary here. Use anyhow!() directly with format string arguments: anyhow!('could not get inode of {}: {}', unshared_namespace_path.path.display(), e).

Copilot uses AI. Check for mistakes.
Comment on lines 204 to 208
return TestResult::Failed(anyhow!(format!(
"could not get inode of {}: {}",
container_ns_path.display(),
e
)));
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The format!() call is unnecessary here. Use anyhow!() directly with format string arguments: anyhow!('could not get inode of {}: {}', container_ns_path.display(), e).

Suggested change
return TestResult::Failed(anyhow!(format!(
"could not get inode of {}: {}",
container_ns_path.display(),
e
)));
return TestResult::Failed(anyhow!(
"could not get inode of {}: {}",
container_ns_path.display(),
e
));

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants