Problems when generating internal types

[cmeeren]

I want to test internal stuff (using InternalsVisibleTo on the main assembly). Specifically, I want FsCheck to generate instances of internal types. This seems to be a bit hit-and-miss, and mostly seems to fail. For example, generating values of an internal union gives SystemArgumentException with message X is an F# union type but its representation is private. You must specify BindingFlags.NonPublic to access private type representations. Generation of some other internal types seem to loop until the test times out and gets aborted.

Is this a known issue? Is it deliberate? Would it be possible to make FsCheck able to generate internal types?

Currently I'm just leaning towards making everything public, but that means the public API of the main assembly is rather more noisy than it needs to be.

[cmeeren]

Nevermind the infinite loop part; it seems that it was caused by FsCheck not picking up internal Arbitrary-returning members on the class I specified in the PropertiesAttribute, and thus not using my designated generators for a few recursive types. (And yes, the members needed to be internal, since they return e.g. ValidCustomer which also needs to be internal since it wraps the internal domain object Customer.)

[kurtschelfthout]

I'm on the fence about getting FsCheck to take internal and private constructors into account. I usually use that to signal that the ctor doesn't do validation or doesn't necessarily uphold an invariant. I assume that the same problem would exist when FsCheck would be trying to call those ctors. So it can cause some confusion - "why isn't FsCheck only using my public ctor, it's generating bogus instances by breaking my encapsulation" type issues.

…

On 4 Oct 2017, at 10:01, Christer van der Meeren ***@***.***> wrote: Nevermind the infinite loop part; it seems that it was caused by FsCheck not picking up internal Arbitrary-returning members on the class I specified in the PropertiesAttribute, and thus not using my designated generators for a few recursive types. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[cmeeren]

In that case surely FsCheck could always use a public ctor if both public and internal ctors exist?

[kurtschelfthout]

In that case surely FsCheck could always use a public ctor if both public and internal ctors exist?

That doesn't seem very transparent either, the generation strategy would change based on access modifiers...

I would consider a configuration option or some such. Unfortunately currently the Config type is a record type, which basically means that every addition to it is a breaking API change. Small mistake, high price 😒

In the shorter term, it would be possible to add an overload of Arb.Default.Derive that takes an option to also use internal/private constructors. You'd then still need to define the arbitrary instances, but they would be a one-liner each.

It probably won't be high in my priority list. I would accept a PR that adds it provided the current behaviour remains the default and the configuration for the new behavior is explicit. It shouldn't be hard to do, some plumbing to get the reflection calls configured correctly.

[ploeh]

I'd like to add a vote against such a feature. Unit tests shouldn't exercise internals of the System Under Test. I think that this is an important enough principle that libraries and frameworks shouldn't support any efforts to do so.

When I still headed AutoFixture, similar requests came up from time to time, and I always tried to engage with people to help them design their software so that it could be tested via its public interface.

I never added a unit-test-the-internals feature to AutoFixture, and I never got the impression that it much hurt adoption.

@cmeeren, there are various ways to address most issues related to testability of a code base, and I'd offer to help. I do think, however, that its a software design issue more than it's an FsCheck issue, so this isn't an appropriate forum for such a discussion.

If you'd like to take me up on the offer, feel free to ask a question on e.g. Stack Overflow or Software Engineering. Just poke me with a link, and I'll be happy to take a look.

[cmeeren]

Thanks @ploeh, I appreciate that. I posted a question on SO at the same time as posting this issue. Basically, I'm marking things internal more or less just because I want to clean up the public API of the assembly. I have followed the suggestion of the currently accepted answer and simply placed all internal code under a namespace called Internal. Feel free to add your thoughts on the underlying issue of whether to have and how to get a clean public API.

[kurtschelfthout]

I have followed the suggestion of the currently accepted answer and simply placed all internal code under a namespace called Internal

That's actually a nice idea. I like Kmett's comment about it, worth linking here: https://www.reddit.com/r/haskell/comments/2nkiiq/testing_internals_without_exposing_them/cmfph90/

[ploeh]

I think, as always, that it's a all a question of context.

My guess is that Kmett's users are sophisticated developers, and there's not that many of them, so doing what he does makes sense in that context. It also helps that he writes Haskell code, where most of the code you write is guaranteed to have no side-effects.

On the other hand, if you're in a situation like Microsoft when they started publishing the .NET framework in the 2000s, a certain degree of paranoia is required regarding invariants and breaking changes.

Most developers probably fall somewhere between two such extremes. There are various axes to consider, such as:

• How many people use your code?
• Do you know who they are?
• Can you get in touch with them if you need to make a breaking change?
• How sophisticated are they?
• Which guarantees can your language offer?
• What's the impact if people misuse your code?

Arriving at the appropriate solution depends on answers to such questions.

[kurtschelfthout]

I was thinking about modules or subsystems that do have a decent interface that allows them to be used correctly by a sufficiently motivated self-learner, but where I (as an open source library author) don't want to commit to keeping the API backwards compatible for all time; writing (a lot of) documentation for it; answer questions about it and as a result put in a lot of time in polishing the API. Something like the Random module is a good example.

[isaevdan]

@kurtschelfthout
I don`t quite understand this part of the message "You must specify BindingFlags.NonPublic to access private type representations".
So is there a workaround how to fix this by specifying any attributes? If not, message seems a bit confusing.
Also I found pull request which seems to do something about it. Using unit test from this PR I see that I can generate my internal DU through Gen.sample. However as soon I try to generate it through method params(using FsCheck.Xunit) it fails with exception above.

[kurtschelfthout]

That message is coming from the reflection API. As a client of FsCheck, you can’t currently pass anything to make it work, see the discussion above.

[isaevdan]

Strange that you still can generate this DU through Gen.sample. Looks like this is implemented technically, just not exposed to parameters generation

[kurtschelfthout]

Some more context is needed here. If you need help with something specific please open a new issue and add a repro.

[teo-tsirpanis]

My opinion is that FsCheck is a general-purpose library, which means that it is used by many people with varying requirements and for varying purposes. Since reflecting over private members can be done with the stroke of a BindingFlag, I see no reason to artificially prohibit this use case in FsCheck, especially since circumventing the invisibility of internal types is possible and supported theoughout the framework with the InternalsVisitedToAttribute.

People mark their types as internal to enforce a boundary around their projects, pave a path of intended use of their libraries, and most importantly, worry less about implementation changes being breaking. Kmett's solution, making everything public but under a namespace called Internal is not common in the .NET world (especially C#) and might alienate some developers, including myself.

I can submit a Pull Request fixing this issue.

[kurtschelfthout]

So I think with the latest PR it already works for F# union and record types at least. As above for general classes with a mix of private and public constructors things can get a bit more hairy.

What is the suggestion more concretely?

[ordinaryorange]

I just hit this issue today.

In my situation I had a F# union with private cases exposed as a c# consumable API.
The union fields are records, and I'd marked these types internal/private as they should not be available publicly.

FSCheck can create an Arbitary of this union type without issue, but I hit an edge case where one of the internal record fields of type was being set to null. (Given the record is internal and only under the control of F# null is an invalid state.)

So I attempted to to register a custom generator for the internal record type to limit the field to non null strings. But the compiler wont let me expose a static method returning an internal type on a public type, and Arb.register needs public static members.

I do agree that something like a BindingFlag option would be helpful to control the search process for registration of arbitrary types. I had a look at V3 and I suspect this edge case will also persist there too.

[kurtschelfthout]

@ordinaryorange this is easy to implement technically (see https://github.com/fscheck/FsCheck/blob/master/src/FsCheck/Arbitrary.fs#L188 ) but it seems to me like you're going to end up with maintaining invariants (in this case, that an F# record type can not be null) twice - once in your actual program and once in your custom generators/shrinkers. This is not a great situation because there is little reason to believe that the two are in sync.

In my experience it's better to generate random input for the constructors of a type - these do not necessarily have to be actual public methods if it's an internal layer, but I mean the constructors that maintain whatever invariants necessary. Presumably your C# consumers have methods or something with which to create the F# union - try building random generators for those instead?

[ordinaryorange]

I take your point, and I agree there is a lot to be said for being strict on testing purely against the public API. I totally get the argument from those on that side of the fence. But I must admit I have hit occasions where I wanted to test the internals of an implementation either to boost confidence in it, or because it is easier than travelling the higher ground.

I tend to agree with @teo-tsirpanis that testing needs vary. Not everything we write is destined for years of production, and if a testing tool can help with the weekend hacks then it is so much more of value.

I'm already sold on FSCheck and if something like BindingFlags does not make it in then no big deal. All worthy of a discussion though.

I do like the idea of nullable reference types. If type string was not be null by default (as is expected in F#), then any generated instances from FSCheck are true strings and in my case I would not have hit the edge case.

[kurtschelfthout]

Ok - that's fair. Like I said it's easy to add, and I'm all for un-opinionated tools. I'll leave it here as a reminder to add.

Nullability - yes it would be great if there was a good way for FsCheck to figure out whether to generate null for ambiguous types like string.