-
Notifications
You must be signed in to change notification settings - Fork 2.2k
adds client api integration tests for runc using bash w/bats #659
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
adds client api integration tests for runc using bash w/bats #659
Conversation
|
@mikebrow simply amazing!. |
|
Looks great! Thanks @mikebrow |
2dde64e to
64c0bab
Compare
|
Looks great. I reckon we should also add some tests making sure that resource limiting actually works (it was broken a few times in the past and nobody noticed until someone tried to vendor it into Docker). |
|
Ayup.. next on my list was actually going to be stress & bootstrap tests.. like constraint checking. First one I started writing I found a bug :-) |
64c0bab to
0a0b04b
Compare
|
rebased on current master... |
|
LGTM |
|
We should add this to the Jenkins test jobs. Aside from my point about the Dockerfile, LGTM. |
| --no-install-recommends | ||
|
|
||
| # install bats | ||
| RUN cd /tmp \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify an exact commit or version in the repo to checkout. This means that we don't have to worry about inconsistencies in the test framework. Please also strip the trailing whitespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the trailing whitespace from this file... Per Michael's comment leaving the which/how of picking or caching a particular version of bats for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think he meant "we should add it to the Jenkins jobs in another PR". I don't like pulling in the latest version of bats from HEAD when doing testing -- that doesn't strike me as being a particularly good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk... np... updated to reset head back to a known commit (which is the latest atm).
Cheers.
|
ya, we can add that in another pr |
0a0b04b to
7f79dcc
Compare
|
Rebased... comment addressed. |
Signed-off-by: Mike Brown <[email protected]>
7f79dcc to
e9f89e1
Compare
|
Comments addressed. |
|
LGTM. |
Closes issue #485
See readme.md for usage instructions.
Signed-off-by: Mike Brown [email protected]