-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Mention in the README.md that InMemorySink.Instance is AsyncLocal #26
Comments
Thanks for the feedback @hiredgunhouse! I’ve not been focusing a lot on the package recently but should have some time soon to add some updates including some better docs around the thread safety and async local stuff. I had been experimenting with something a bit more robust but that proves very difficult depending on test framework as well as the logger itself but I agree better docs would be helpful 👍 |
I'm also using I ended up creating a separate instance of |
That is the better approach and guarantees you’ve got isolated instances. |
I tried to use the static
Maybe there is a better way... In any case, great sink! |
It is not mentioned in the README.md that InMemorySink.Instance is actually AsyncLocal which is quite important piece of information. I thought that InMemorySink.Instance is buggy when it was not behaving as I was expected: Instance is typically a singleton. I only discovered that important fact through issues.
Also you could mention that it is possibly important if you mix InMemorySink with integration tests based on WebApplicationFactory, that was my use-case where the AsyncLocal completely surprised me.
BTW. So far I love it, thank you for your contribution, InMemorySink combined with the assertion library turns tests into little things of beauty :)
The text was updated successfully, but these errors were encountered: