-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Update registrar unit tests to match them of cri-o #2841
Conversation
Hi @saschagrunert. Thanks for your PR. I'm waiting for a containers or openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hm, the issue with the CI looks like a bug in ginkgo: onsi/ginkgo#427 🤔 |
/hold |
7d2ffa6
to
057aa35
Compare
why do we need this? |
The unit tests for this package have been upgraded within cri-o to use ginkgo/gomega (higher test coverage and bdd style). I thought it would be nice to have them here as well, since cri-o could rely completely on the package within libpod and remove its own copy of the code. |
Tests are not working... |
- Add the test framework abstraction - Update the unit tests to run with ginkgo Signed-off-by: Sascha Grunert <[email protected]>
057aa35
to
88b0e74
Compare
I had to exclude the pkg/apparmor tests until onsi/ginkgo#570 is not merged yet. This makes me not happy at all, WDYT? If you tend to not go this way here I would also be fine in closing this PR. |
/hold cancel |
Personally, I find the standard |
LGTM |
/ok-to-test |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Hey, I basically copied the registrar tests from cri-o over here including their dependencies:
As a follow-up I would remove the registrar completely from cri-o and use libpods' one.
Another general improvement could be to utilize the already vendored ginkgo sources for running the tests, like we do in cri-o:
This would make the test results more reliable.