youki
youki copied to clipboard
Add linux_devices test
Adds the linux_devices integration test.
Codecov Report
Merging #2708 (b8bf9a1) into main (cfa2ea9) will not change coverage. Report is 2 commits behind head on main. The diff coverage is
n/a
.
Additional details and impacted files
@@ Coverage Diff @@
## main #2708 +/- ##
=======================================
Coverage 65.41% 65.41%
=======================================
Files 133 133
Lines 16942 16942
=======================================
Hits 11082 11082
Misses 5860 5860
Hey, due to a bug in our CI, currently the tests are not validated on runc. May I ask you to wait for https://github.com/containers/youki/pull/2707 to get merged and rebase on it, so we know that the added tests also work with runc? Thanks :)
It is merged now, So can you rebase and push?
Thank you. Should be rebased now.
Hello, yes this is a straightforward port from the test in oci-runtime tools and there is another test validateDefaultDevices that seems to use this function as well (in oci-runtime tools).
Hello, yes this is a straightforward port from the test in oci-runtime tools and there is another test validateDefaultDevices that seems to use this function as well (in oci-runtime tools).
Ok, so do you think we should separate both of these, and have smaller, maybe more modular functions that do tests specific to their test cases? Or this is the best solution, and breaking up function would be worse code than this? This does not have to be done in this PR, just want to know your thoughts.
@omprakaash ping!
Hey sorry for the delay. Will get back to this tomorrow.
Hey I believe that this is fine, since these are used only for testing and the other test when added, could just call this function. Having separate functions for each type seems unnecessary to me since a lot of the logic seems to overlap for multiple device types, but happy to change this if you strongly feel otherwise.
Hey I believe that this is fine, since these are used only for testing and the other test when added, could just call this function. Having separate functions for each type seems unnecessary to me since a lot of the logic seems to overlap for multiple device types, but happy to change this if you strongly feel otherwise.
Fair enough. I don't have any better idea either, so if in future this proves to be an issue, we can fix ti then. Let's keep it as is now :+1:
Hey I mad a minor change where we don't return immediately from check function if permissions are not configured, it is similar to how uid/gid validation is done in our implementation.