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

InternalsVisibleTo unit tests #14520

Closed
krwq opened this issue May 1, 2015 · 4 comments
Closed

InternalsVisibleTo unit tests #14520

krwq opened this issue May 1, 2015 · 4 comments
Labels
design-discussion Ongoing discussion about design without consensus
Milestone

Comments

@krwq
Copy link
Member

krwq commented May 1, 2015

What do you think, is it ok to use InternalsVisibleTo in unit tests or is it better to do special (sometimes quite heavy) set up?

My vote is for "it is ok" as I believe it would make tests much cleaner, easier and faster and it doesn't make sense to over complicate just for the sake of not using InternalsVisibleTo.

@bpschoch
Copy link
Contributor

bpschoch commented May 1, 2015

IMHO, it depends on the philosophy of testing. If unit testing is to only test the public interfaces, then there is no need to access internal items. On the other hand if testing is there to exercise all of the internal workings of a component, then perhaps it's justified.

I'm wrestling with that issue right now with the testing I'm setting up with System.IO.Pipes. So far I'm only using public interfaces, however it's not possible to really to get full code coverage without accessing some of the internal components.

@krwq
Copy link
Member Author

krwq commented May 1, 2015

@bpschoch, If some code paths are not accessible that means that we have some dead code which in most cases we can remove. If it is accessible but the set up is complicated or quite heavy (consuming unnecessary resources or takes a long time or really long piece of code initializing) my opinions is to use InternalsVisibleTo. If accessible and set up is fairly easy (few lines of code which do no heavy ops) always use that.

@weshaggard
Copy link
Member

weshaggard commented May 2, 2015

I generally diss-like InternalsVisibleTo just for the purpose of code coverage. I believe it causes inconsistency and unnecessary coupling that makes the tests too fragile. For example if you call an internal API you are taking a dependency on a particular implementation detail that may get you into a state that isn't consistent with the state you would be in if you only called through public APIs and so you aren't actually testing the true behavior. As for the coupling you are tying the set of tests to one given implementation. Using the public API contracts allow you to have a consistent set of tests that can be ran on the various implementations (i.e .NET Core, .NET Framework, .NET Native, etc) to ensure they behave consistently and by depending on internals you restrict the tests to only one such implementation.

@sharwell
Copy link
Member

@weshaggard From the perspective of public APIs, you are absolutely correct. However, public APIs are not the only boundary within an application or component for which you want to have well-defined and well-tested behavior at the boundary. IVTs for testing are required for testing internal APIs representing a component or module boundary (e.g. an internal interface which another developer might interact with, but not know the implementation details for that interface). Regardless of what code coverage tooling says, a test suite is crippled if it's incapable of testing internal abstractions.

📝 Note that I'm not advocating for fine-grained unit tests here. That is a totally separate discussion.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design-discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests

6 participants