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.

Sign up for GitHub

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

Jump to bottom

Add test to ensure the the logs are outputted with zap in the manager #1739

Closed
camilamacedo86 opened this issue Oct 23, 2020 · 2 comments
Closed

Add test to ensure the the logs are outputted with zap in the manager #1739

camilamacedo86 opened this issue Oct 23, 2020 · 2 comments
Labels
testing

Comments

@camilamacedo86
Copy link
Member

What do you want to happen?
It is a follow up of #1721. Would be nice to add a test to ensure that the logs are outputted with zap as described in #1721 (comment)

Notes

  • The change was done only for v3+ so the test should only cover the projects built with v3-alpha plugin.
@camilamacedo86 camilamacedo86 added kind/feature Categorizes issue or PR as related to a new feature. testing and removed kind/feature Categorizes issue or PR as related to a new feature. labels Oct 23, 2020
@estroz
Copy link
Contributor

estroz commented Oct 27, 2020

Why do we need to test the output format of an external package for correctness here? This should be done in controller-runtime.

@camilamacedo86
Copy link
Member Author

HI @estroz,
The idea here is only to ensure that the controller-runtime/manager is setup with the zap properly and not ensure that it works as expected. However, you are right it might be too much for e2e. So, I am closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
testing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@camilamacedo86 @estroz