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

Logging providers cause issues in unit test context #1

Closed
AlexEngblom opened this issue Feb 24, 2020 · 5 comments
Closed

Logging providers cause issues in unit test context #1

AlexEngblom opened this issue Feb 24, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@AlexEngblom
Copy link

We use NLog in our solution. It seems that LibLog resolves and initializes NLog provider with the configuration we have in the project root. This means that Malware scanner tries to internally write according to our default nlog configuration when running unit tests. This even causes tests to crash when they try to resolve NLog db connection while lacking permissions.

As a workaround we clear the NLog configuration via static access point at the start of unit test
LogManager.Configuration = new NLog.Config.LoggingConfiguration();

However I think this is an ugly and unwanted side effect.
Maybe this could be avoided by using ILogger interface and dependency injection approach instead. Let the software always decide the implementation instead of sniffing for usable providers.

@flytzen
Copy link
Member

flytzen commented Feb 25, 2020

That is a good point; LibLog has now been deprecated in favour of ILogger (damianh/LibLog#270) so it's time for this package to change with the times.

@flytzen flytzen added the enhancement New feature or request label Feb 25, 2020
@flytzen
Copy link
Member

flytzen commented Mar 2, 2020

Note to self; inject ILogger, not ILogger<T> as per https://blog.rsuter.com/logging-with-ilogger-recommendations-and-best-practices/

@flytzen flytzen mentioned this issue Mar 3, 2020
@flytzen
Copy link
Member

flytzen commented Mar 3, 2020

@AlexEngblom I've pushed a 1.0.0-beta1 version here: https://www.nuget.org/packages/MalwareScan.AMSI/1.0.0-beta1 (may be a few minutes before it shows up). I have simply removed LibLog as I am not sure there is that much value in logging?
Would you mind taking it for a spin and let me know if it suits? I'll push a format v.1 after that.

@AlexEngblom
Copy link
Author

AlexEngblom commented Mar 11, 2020

Works without issues, thank you. I suppose logging is not a mandatory, however it would be nice to have a way to see what happened under the hood (detection information). The result for .HasVirus() is just a boolean, so it doesn't give that much information of what was detected and so on.

@flytzen
Copy link
Member

flytzen commented Mar 27, 2020

Thanks @AlexEngblom; I have pushed v1 now.
I have opened another issue #3 to add support for ILogger.

@flytzen flytzen closed this as completed Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants