Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented util function to test in child process #345

Merged
merged 4 commits into from
Sep 30, 2021

Conversation

yihuaf
Copy link
Collaborator

@yihuaf yihuaf commented Sep 29, 2021

We have many cases where we want to write unit test in a child process, because the functions we want to test will change the underlying OS state. Since cargo test runs in a single process, we have to fork unto child process to run functions. Before, using only fork, we can only pass int back to parent process. Using ipc-channel crate, we can pass richer test results back to the parent process so the error message is much clearer.

I only made change to seccomp tests as an example. Let me know what you think.

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2021

Codecov Report

Merging #345 (6d4bda6) into main (884428a) will increase coverage by 0.07%.
The diff coverage is 80.76%.

@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   70.38%   70.45%   +0.07%     
==========================================
  Files          46       46              
  Lines        6790     6844      +54     
==========================================
+ Hits         4779     4822      +43     
- Misses       2011     2022      +11     

@yihuaf yihuaf requested a review from utam0k September 29, 2021 06:05
Cargo.toml Outdated
@@ -50,6 +50,7 @@ tabwriter = "1"
fastrand = "1.4.1"
crossbeam-channel = "0.5"
seccomp = { version = "0.1.0", path = "./seccomp" }
ipc-channel = "0.15.0"
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to make ipc-channel depend on only dev-dependencies, is it possible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes it can be done. Take a look at the latest commit. I may want to use ipc-channel in other part of Youki's code, but let's keep it as dev-dependencies for now, and discuss this when we get there.

@utam0k
Copy link
Member

utam0k commented Sep 29, 2021

I've been trying to work on this issue too to see if I can't do something about it. I agree with you about putting this processing together.

@yihuaf yihuaf requested a review from utam0k September 29, 2021 23:08
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

Thanks!

@utam0k utam0k merged commit c206d88 into containers:main Sep 30, 2021
@yihuaf yihuaf deleted the yihuaf/seccomp branch September 30, 2021 06:59
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.

None yet

3 participants