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.

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

Can't mock logger anymore #444

Open
smacker opened this issue Jan 4, 2019 · 5 comments
Open

Can't mock logger anymore #444

smacker opened this issue Jan 4, 2019 · 5 comments
Labels
enhance-tests This will improve the app testing

Comments

@smacker
Copy link
Contributor

smacker commented Jan 4, 2019

After #439 it's impossible to mock logger in tests anymore.

@carlosms
Copy link
Contributor

carlosms commented Jan 4, 2019

Is there any more context? are tests failing? Any specific place where we need to mock a logger?

@smacker
Copy link
Contributor Author

smacker commented Jan 4, 2019

screenshot 2019-01-04 at 14 37 08

Tests are failing obviously.

@smacker
Copy link
Contributor Author

smacker commented Jan 8, 2019

As a solution, we can do in ctxlog/context.go:

var NewLogger = log.New

then we will be able to mock it in the tests. What do you think @carlosms ?

@carlosms
Copy link
Contributor

It looks risk... but we can go that way to unblock the issue and maybe rethink later if we find a better solution.

Can't we change the go-log DefaultFactory instead? I think it should be picked by ctxlog.Get -> log.New

@smacker
Copy link
Contributor Author

smacker commented Jan 11, 2019

Sadly we can not. LoggerFactory isn't an interface but a struct defined in go-log. And there is no way to replace logrus in New with something else.

I'll proceed with the solution from above to unblock related PR but keep this issue open because I don't like this solution too much.

@dpordomingo dpordomingo added enhancement New feature or request enhance-tests This will improve the app testing and removed enhancement New feature or request labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhance-tests This will improve the app testing
Projects
None yet
Development

No branches or pull requests

3 participants