-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add the most basic sniff tests of runc #554
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
Conversation
7f82a80 to
99a9663
Compare
|
Ironically, the tests don't pass. 😸 |
|
yea its because it hangs for some reason on janky but not locally - could be a sign of an issue.... |
|
I would like to see this to be something like runc level integration test, which can test every runc command with all kinds of different configs. This is a start which is good 👍 but I don't think it's a good framework to be expanded on :( |
|
@hqhq agreed, which is why I said "I expect this to be removed once we have a real testing infrastructure." |
|
LGTM |
|
Fairly large binary in repo for thing which will be removed :) I think download it would be lesser evil, also I think that libcontainer tests already download it somewhere. |
|
+1 on downloading the tar of busybox (just steal the script from Docker). Apart from that, this looks like a good first step on improving our testing. We still need to add more tests that actually check that resource limits are enforced. |
4b0b083 to
9e73010
Compare
|
remove the tar file - downloaded it from dockerhub instead |
|
#600 fixes the test issue - its unrelated to this PR. |
just so that we're not merging code into master w/o any tests at all. I expect this to be removed once we have a real testing infrastructure. Signed-off-by: Doug Davis <[email protected]>
|
rebased |
|
LGTM |
Add the most basic sniff tests of runc
just so that we're not merging code into master w/o any tests at all.
I expect this to be removed once we have a real testing infrastructure.
Signed-off-by: Doug Davis [email protected]