xUnit integration: Automatically discover Arbitraries from parameter types

[fsoikin]

In my C# land of doom, I find myself following this pattern rather frequently: have my tests accept a data structure as "input", and have that data structure come with its own generator (see code sample below).

To do this, I have to always specify PropertyAttribute.Arbitrary with C#'s clunky array + typeof syntax, which gets annoying. And it gets even more annoying when I start reusing these data structures in multiple tests.

At the same time, it's relatively easy to just have the PropertyAttribute automatically pick up types of method parameters and treat them as if they were added to PropertyAttribute.Arbitrary.

I have a working solution (complete with a unit test) which I can push, provided you consider this new addition worthy. Let me know.

[Property( Arbitrary = new[] { typeof(Args) } )]
public void Sould_do_awesome_things( Args args ) {
   ...
}

class Args {
   public int X;
   public string Y;

   public static Arbitrary<Args> GenArgs() {
      return (
         from x in Gen.Choose( 42, 1024 )
         from y in Arb.Generate<string>()
         select new Args { X = x, Y = y } )
         .ToArbitrary();
   }
}

P.S. It would be nice to have FsCheck derive the whole Args for me, same way it does for F# native structures, but that's a story for next time.

[ploeh]

Why don't you just derive from [Property] in order to specify any custom Arbitraries, like I did here?

type Letters =
    static member Char() = Gen.elements ['A' .. 'Z'] |> Arb.fromGen

type DiamondPropertyAttribute() =
    inherit PropertyAttribute(
        Arbitrary = [| typeof<Letters> |],
        QuietOnSuccess = true)

[<DiamondProperty>]
// Test goes here...

While the above example is in F#, it relies on inheritance, so should also work from C#.

[fsoikin]

I don't derive from PropertyAttribute, because I have different arbitraries every time.
It's not a limited set that I use often. They are mostly local to tests, with occasional reuse.

[fsoikin]

Also, by the way, in your sample you could have used the [<Arbitrary>] attribute, apply it to the whole module:

module Ploeh.Samples.DiamondProperties

open System
open FsCheck
open FsCheck.Xunit
open Ploeh.Samples

type Letters =
    static member Char() = Gen.elements ['A' .. 'Z'] |> Arb.fromGen

[<Arbitrary([| typeof<Letters> |])>]
module Tests  =

    let ``Diamond is non-empty`` (letter : char) =
        let actual = Diamond.make letter
        not (String.IsNullOrWhiteSpace actual)

    let split (x : string) =
        x.Split([| Environment.NewLine |], StringSplitOptions.None)

    let trim (x : string) = x.Trim()

    let ``First row contains A`` (letter : char) =
        let actual = Diamond.make letter

    ...

[ploeh]

@fsoikin, nice tip; I din't know about that one. Thank you for sharing :+1:

[kurtschelfthout]

I'm a bit on the fence on this one, not sure if there should be yet another way of registering Arbitrary instances.

Can you explain why either attributing the module or calling Arb.register in some setup method does not work for you?

Another thing that I find a bit weird about this pattern - though I just may be misunderstanding you - is that this would need your actual code to take a dependency on FsCheck, to be able to add the generator method there?

[fsoikin]

Attributing the module works, but it is verbose: I need to type in all types twice - once in [Arbitrary] on the module (or in [Property] on the method), and another time in the method's parameter list.

Arb.register has the same disadvantage, but additionally it's unclear where to put the call. I tried to put it in static initializer of Args, but apparently it doesn't get called before PropertyAttribute is asked to enumerate test commands.

My production code (I will assume that by "actual" you mean "production") does not need to take dependency on FsCheck. All this, including Args, happens in tests[1].

Think of it this way: this change does not affect core functionality, does not disrupt anything outside of the PropertyAttribute-decorated tests, but at the same time provides a small yet consistent convenience that will go a long way towards converting infidels. :-)

---

[1] Some background: since I'm in C# land, everything is kinda object-oriented and interface-abstracted (or at least heavily inclined that way), so that setting up involves pushing stuff to various containers and mutating various variables, which FsCheck's core can't do, so instead I create this Args structure that describes initial state in a pure-data way, then have FsCheck generate it for me, and then the actual test method does pushing and mutating. Think of it as poor man's model-based testing.

[ploeh]

What I usually do in C# is to design the SUT and the input that goes into it in such a way that it's fairly easy for an automated tools like AutoFixture to just generate valid instances of the appropriate objects. In FP, this approach is what Scott Wlaschin calls making illegal states unrepresentable; in OO, this is known as encapsulation.

As an example, this works with AutoFixture:

public class MyClass
{
    public String Foo { get; set; }
}

public class Tests
{
    [Theory, AutoData]
    public void UsingAutoFixture(MyClass sut)
    {
        // Test goes here...
    }
}

However, with FsCheck, to my surprise, it doesn't:

public class MyClass
{
    public String Foo { get; set; }
}

public class Tests
{
    [Property]
    public void UsingFsCheck(MyClass sut)
    {
        // Test goes here...
    }
}

This produces this error message:

Test 'ClassLibrary1.Tests.UsingFsCheck' failed: System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
---- System.Exception : Geneflect: type not handled ClassLibrary1.MyClass
    at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
    at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
    at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
    at FsCheck.Runner.checkMethod(Config config, MethodInfo m, FSharpOption`1 target)
    at <StartupCode$FsCheck-Xunit>.$PropertyAttribute.EnumerateTestCommands@83.Execute(Object testClass)
    at Xunit.Sdk.FixtureCommand.Execute(Object testClass)
    at Xunit.Sdk.BeforeAfterCommand.Execute(Object testClass)
    at Xunit.Sdk.LifetimeCommand.Execute(Object testClass)
    at Xunit.Sdk.ExceptionAndOutputCaptureCommand.Execute(Object testClass)
    ----- Inner Stack Trace -----
    at FsCheck.ReflectArbitrary.reflectObj@95-4.Invoke(String message)
    at FsCheck.Common.f@1[a,b](IDictionary`2 memo, FSharpFunc`2 f, a n, Unit _arg1)
    at FsCheck.Common.memoizeWith[a,b](IDictionary`2 memo, FSharpFunc`2 f, a n)
    at FsCheck.Common.memoize@32.Invoke(a n)
    at FsCheck.ReflectArbitrary.reflectGen[a](FSharpFunc`2 getGenerator)
    at FsCheck.Arb.Derive@789.get_Generator()
    at FsCheck.Testable.forAll@170.Invoke(Unit unitVar)
    at FsCheck.GenBuilder.delay@56.Invoke(Int32 n, StdGen r)
    at FsCheck.Runner.test@102-1.GenerateNext(IEnumerable`1& next)
    at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
    at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.System-Collections-IEnumerator-MoveNext()
    at Microsoft.FSharp.Collections.SeqModule.TakeWhile@1490.GenerateNext(IEnumerable`1& next)
    at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.MoveNextImpl()
    at Microsoft.FSharp.Core.CompilerServices.GeneratedSequenceBase`1.System-Collections-IEnumerator-MoveNext()
    at Microsoft.FSharp.Collections.SeqModule.Fold[T,TState](FSharpFunc`2 folder, TState state, IEnumerable`1 source)
    at FsCheck.Runner.runner[a](Config config, a prop)
    at FsCheck.Runner.check[a](Config config, a p)

That's quite surprising, but I have to admit that I've never tried to use FsCheck with classes from C# before now. Doesn't FsCheck create instances of classes?

[moodmosaic]

Indeed, this looks quite surprising.

IMO, it would be nice to have some kind of fallback mechanism using a reflection-based Arbitrary and having its Generators automatically registered.

It probably wouldn't make sense to support Shrinking for these scenarios.

---

That way, it'd be possible to have the following working out of the box:

F#

type MyClass () =
    member val Foo = Unchecked.defaultof<string> with get, set

type MyOtherClass () =
    member val Bar = Unchecked.defaultof<double> with get, set

module Tests =
    [<Property>]
    let ``Test MyClass using FsCheck.Xunit`` (sut : MyClass) =
        ()

    [<Property>]
    let ``Test MyOtherClass using FsCheck.Xunit`` (sut : MyOtherClass) =
        ()

C#

public class MyClass
{
    public string Foo { get; set; }
}

public class MyOtherClass
{
    public int Bar { get; set; }
}

public class Tests
{
    [Property]
    public void TestMyClassUsingFsCheckXunit(MyClass sut)
    {
    }

    [Property]
    public void TestMyOtherClassUsingFsCheckXunit(MyOtherClass sut)
    {
    }
}

Instead of having to setup and register Arbitraries for each class:

F#

type MyClass () =
    member val Foo = Unchecked.defaultof<string> with get, set

type MyOtherClass () =
    member val Bar = Unchecked.defaultof<double> with get, set

type Arbitraries =
    static member MyClass() =
        Arb.generate<string>
        |> Gen.map (fun x -> MyClass(Foo = x))
        |> Arb.fromGen

    static member MyOtherClass() =
        Arb.generate<double>
        |> Gen.map (fun x -> MyOtherClass(Bar = x))
        |> Arb.fromGen

[<Arbitrary([| typeof<Arbitraries> |])>]
module Tests =
    [<Property>]
    let ``Test MyClass using FsCheck.Xunit`` (sut : MyClass) =
        ()

    [<Property>]
    let ``Test MyOtherClass using FsCheck.Xunit`` (sut : MyOtherClass) =
        ()

C#

public class MyClass
{
    public string Foo { get; set; }
}

public class MyOtherClass
{
    public int Bar { get; set; }
}

public class MyArbitraries
{
    public static Arbitrary<MyClass> MyClass()
    {
        return Arb
            .Generate<string>()
            .Select(x => new MyClass { Foo = x })
            .ToArbitrary();
    }

    public static Arbitrary<MyOtherClass> MyOtherClass()
    {
        return Arb
            .Generate<int>()
            .Select(x => new MyOtherClass { Bar = x })
            .ToArbitrary();
    }
}

public class Tests
{
    static Tests()
    {
        Arb.Register<MyArbitraries>();
    }

    [Property]
    public void TestMyClassUsingFsCheckXunit(MyClass sut)
    {
    }

    [Property]
    public void TestMyOtherClassUsingFsCheckXunit(MyOtherClass sut)
    {
    }
}

[fsoikin]

@ploeh, making illegal states irrepresentable is a swell idea, and I usually do that when I can. However, I am constrained by preexisting parts of the system, which I can't just redesign from scratch. For example, there is a relational database accessed through ORM, and data access code needs to be tested. This means that, as part of my test, I need to mock up relevant parts of the database, which is the very "pushing and mutating" I was talking about.

@moodmosaic, FsCheck already does reflection-based deriving, but it is explicitly limited to F# native structures (records, unions, enums, tuples).

It also has some support for C#-style records, but it is very limited. In order to qualify, type must meet the following:

• Be a non-abstract, non-generic class
• Have exactly one public constructor
• Have no writable properties or fields (i.e. all properties must have { get; }, but not { set; }, even a private one, and fields must be readonly)

In other words, this will work:

        public class X
        {
            private readonly int _y;
            public int Y { get { return _y; } }

            public X( int y ) { _y = y; }
        }

Which is, quite obviously, not very practical. This problem, I was going to tackle next.

[mausch]

Please see http://bugsquash.blogspot.com/2014/02/generating-immutable-instances-in-c.html for some rationale on the rules to generate "C# instances" automatically. IMHO allowing any kind of classes to be automatically generated will open the doors for ambiguities and therefore WTF moments. I'd rather keep things safe by default. Be explicit about what you want to generate and how. It's just a few lines of code.

While I'm here I'll also add that this seems like another limitation of attribute-based test frameworks. With Fuchu it would be trivial to just call Arb.register before calling run on the tests and get global generators. I wouldn't recommend it though, I'd much rather keep generators as local as possible, as a change in a global generator will likely break some tests. If you want to keep Xunit I'd look into integrating FsCheck with @ploeh 's Exude in a similar way to Fuchu's integration

[fsoikin]

@mausch, I agree that FsCheck shouldn't blindly and automatically derive all .NET types. However, there would nothing wrong with doing some automaton when explicitly asked to do it. For example:

public class X {
   public int A { get; set; }
   public string B { get; set; }

   public static Arbitrary<X> Gen = Arb.DeriveProperties<X>();
}

[mausch]

Ah, sorry, I think it's ok if you explicitly call a method to derive the Arb instance. I thought this was about enhancing the built-in reflection-based generator.

[moodmosaic]

@mausch wrote:

allowing any kind of classes to be automatically generated will open the doors for ambiguities [..]

OTOH, I think, it might be scary for new FsCheck users, if they can't just use it on plain-C# DTOs like:

public class MyClass
{
    public string Foo { get; set; }
}

I could imagine a new FsCheck user saying Hmm, can't FsCheck just create a simple DTO class? :anguished:

[kurtschelfthout]

@fsoikin I see. Can you explain why don't you use model-based testing? https://fscheck.github.io/FsCheck/StatefulTesting.html I would love contributions/suggestions to extend the Command/CommandGenerator API, rather than poor man's solutions.

Sorry if you think I'm being reluctant. Just trying to get the full picture here.

Agree with @mausch that the philosophy of FsCheck should be to keep automatic generation reasonable - imo that means limiting it to immutable, algebraic data types, pretty much, since those have a solid foundation. If you want to test objects with mutable state, you need model based testing (which FsCheck also supports, albeit somewhat limited).

That said fully agree that what we have currently is not yet all the way there (e.g. private setters should not be a problem), but various bits of FsCheck, do assume that the generated values are immutable. If they aren't we're in la-la land as far as semtantics are concerned (think replay, shrinking, generating to a certain extent, gatherring statistics etc).

@moodmosaic I'm going to make a stand here: I don't want to lose simple for easy. If the data is not immutable, and FsCheck generates it automatically, that gives the wrong impression. Yeah it might initially work (oh, how easy!), but it can give more subtle interactions later on, because it's not simple to reason about these types.

[mausch]

:+1: @kurtschelfthout

[fsoikin]

@kurtschelfthout:

I do not use model-based testing, because right now it's a royal pain from C#. Also, as you note, it does not support everything. In particular, most of my tests require "setup-act-assert" pattern, and current FsCheck support only provides the latter two steps. That's why I use poor man's solution to generate the "setup" part.

I do understand that you would love contributions and suggestions (that's the very reason I filed this very issue in the first place), but those are orders of magnitude harder to do than poor man's solutions on the spot. To be sure, I will get to them once I accumulate enough material to draw inferences (i.e. I'm not yet sure what suggestions to make exactly), but right now I need to crank out production code to make my customers happy.

And on autogenerating mutable values: while I do understand your sentiment about giving the wrong impression, I think it's too late for that, the impression is already there. Think about it: FsCheck does already have the ability to generate mutable data, you just have to write a lot of boilerplate for that. And FsCheck doesn't complain about it in any way, although theoretical possibility exists to do so.

And so here's the situation: there is this thing that people actually do, and it works perfectly fine, but requires a lot of boilerplate/repeated code, and is easily automatable, but we don't automate it, because it would destroy the purity that we don't have anyway. Huh?

And just after I typed all that, an idea occurred to me how to address your concerns and still have my cake and eat it, too: use an immutable interface! How about having the user declare data as an interface, and then have FsCheck generate implementation:

public interface Data {
  int X { get; }
  string Y { get; }
}

public static Arbitrary<Data> Gen = Arb.DeriveInterface<Data>();

Sure, this interface technically allows mutable implementations, but FsCheck can guarantee that its own autogenerated ones are immutable. Which is a stronger guarantee than that provided by the monadic syntax for generators.

A downside would be the need to dynamically generate types, but fortunately, that's not very hard these days.

[ploeh]

Since @fsoikin was kind enough to point out that C# classes are supported, as long as they meet certain requirements, instead of Arbitraries, you could create Test Data Builders in your C# test libraries. This is a trick I sometimes use when I have types that reflection-based tools like FsCheck or AutoFixture can't deal with in a satisfactory manner.

As an example, given the SUT MyClass:

public class MyClass
{
    public string Foo { get; set; }
}

you can, in your test library, define an immutable Test Data Builder than FsCheck can generate:

public class MyClassBuilder
{
    private readonly string foo;

    public MyClassBuilder(string foo)
    {
        this.foo = foo;
    }

    public string Foo { get { return this.foo; } }

    public MyClass Build()
    {
        return new MyClass { Foo = this.foo };
    }
}

This would allow you to write an FsCheck property like this:

public class Tests
{
    [Property]
    public void UsingFsCheck(MyClassBuilder sutBuilder)
    {
        MyClass sut = sutBuilder.Build();
        // Test goes here...
    }
}

FsCheck is going to execute the UsingFsCheck property a hundred times, so you'll have 100 different instances of MyClassBuilder, and thus, 100 different instances of MyClass.

You can include as much, or as little, behaviour and functionality into Test Data Builders as you need. This means that even if your SUT is a legacy class that FsCheck can't easily deal with, you can always create a Test Data Builder that builds the SUT according to that SUT's particular rules.

This is possible today with FsCheck.

---

As a side note, I think it'd be nice to keep FsCheck as 'pure' as possible. That's the reason I so enjoy working with it. For all the impure stuff, I already have AutoFixture, which will fill in writable properties, etc. :stuck_out_tongue_winking_eye:

[fsoikin]

But this is so much noise! Look: foo is mentioned 8 (!!!) times on 5 lines. What happens when I have 5 such properties? 10? Lists, dictionaries? Boilerplate, boilerplate everywhere!

[ploeh]

True, it would have been terser in F#. :wink:

Often, I find that when a tool resists me, it prompts me to reconsider my current approach.

• Is it the tool at fault because it can't address my problem?
• Or could it be that I ought not to have that problem in the first place?

In my experience, that sort of friction is a benefit of a well-designed and consistent tool. It forces me to think, so I'd be unhappy if it stopped resisting my bad design approaches. Such tools shouldn't make it too easy to work around bad design decisions.

When it comes to legacy code, obviously it's not your fault if you inherited something that was badly designed. Still, if a tool adds 'support for bad design', it becomes a less valuable tool. It may solve a particular problem someone is having, but it makes it worse for everyone else.

[kurtschelfthout]

Some thoughts.

1. I would like to find out why you think model based testing is a "royal pain". I think it's very much like your proposed approach in syntax at least which seems to be what you care about. I do have to update the doc; the page I linked to is grossly out of date. Asof 2.0.1 this is the state of the art: https://github.com/fscheck/FsCheck/blob/master/docs/csharp/StatefulTesting.cs Note that there is a setup phase (InitialActual and InitialModel).
2. There is a difference between "allowing" a certain way of working and endorsing it; it's impossible to prevent every potentially problematic usage, but it is possible to make it not straightforward. Making a potentially problematic pattern easy would be a disservice to the user (choosing easy over simple). Ultimately, this is a matter of taste.
3. I'm not clear why the interface would be better than creating the immutable type and having FsCheck generate that - slightly shorter because you don't have to write a constructor?
4. I feel bad for dismissing many ideas here without coming up with at least one you can dismiss too :) But coming at it from model based testing, how about a simple wrapper around model based testing that automatically generates "setter-commands", i.e. commands that set mutable properties to an arbitrarily generated value, and take a property to check after each set. And if the object has a parameter-less constructor, maybe you don't even need to provide an "initial actual instance". Or a model, if you don't need it.

[fsoikin]

It looks like this discussion is getting derailed more and more. I'd like to point out that this issue is not actually about automatically deriving C#-style POCOs, but about adding some automation to PropertyAttribute. I wasn't prepared to have a discussion about POCOs, I merely hinted at it, and then followed up because both @ploeh and @moodmosaic talked about it.

Now, responding to thoughts:

1. On model-based testing:
   • Using it from C# is a royal pain, because it requires creating ICommand implementations as separate classes. Is there, perhaps, a better way? If there is, I was unaware of it. If there is not, I can propose one.
   • I was, perhaps, not clear enough when I mentioned lack of setup phase. What I meant to say was, setup phase is not arbitrary (or at least I don't see how to make it so). One may argue that having arbitrary setup phase is equivalent to having a constant one and then executing some arbitrary commands, but it is not always so: sometimes, a large system consists of multiple components, some of which collect data, others transform, and yet others query, and I would like to test these components in isolation, away from the rest of the system.
2. Ok, ok, got it: no automatic deriving of C# mutable POCOs. See above.
3. Interface is not only shorter by one constructor declaration, but also constructor body and private property-backing fields. All in all, every record member, needs to be mentioned four times minimum: property declaration, backing field declaration, constructor parameter, and assignment of parameter to field within the constructor body. See example in my comment above. Interface lets me to mention each property only once. Bonus - no repeated public on every line.
4. I actually feel like I've been dismissing things myself :-) As for the wrapper idea - I was under impression that you were against the very idea of automatically generating POCOs, regardless of implementation. Did I misunderstand?

[kurtschelfthout]

PS I updated the documentation at https://fscheck.github.io/FsCheck/TestData.html

[kurtschelfthout]

Sorry for the delayed reply.

The discussion about POCOs is important because they are the justification for making changes here. So I don't think that's a derailment in that sense.

1. I don't think there are all that many better options to creating classes. We can create a command that takes a bunch of Funcs so you don't have to implement a class, not sure that is going to be that much better. But it doesn't cost much to have it so am happy to accept a PR in that vein. We can create reflective command generators for getters, setters and some methods (that will be weaker than the ones you create yourself - they can only execute code, not derive post conditions, clearly). But it is still useful - especially in combination with contracts and some specified invariants.

The setup phase cannot be arbitrary because at each step there is a model with which you can check the expected behavior of the class. This is unrelated to whether or not you can test a class separately from the rest of the system.

1. The verbosity you're describing is entirely due to C#'s class declaration syntax. That will hopefully get better over time, but I hardly think it's worth putting workarounds in. Time is better spent elsewhere.
2. I am in favour of making it easier to use model-based testing, which will include making it easier to generate POCOs by starting from an "empty" instance and the applying methods to it - the latter we can do reflectively so it should be pretty lightweight. The gains are that we now turn an arbitrary POCO into a well-defined empty poco + a list of applied methods. We gain shrinking and replay-ability. (Of course, there can be other sources of side-effects, like statics and threading, that are not taken into account. This is already the case. This is not Haskell.).

[kurtschelfthout]

Re: point 1. There is already Command.Create to create a command without implementing a class. I totally forgot about that. https://github.com/fscheck/FsCheck/blob/de37c297b21ef2ee9099c422db65992a75201caf/examples/FsCheck.MsTest.Examples/ClassesToTest/CounterSpec.cs

[kurtschelfthout]

Closing this (now inactive) discussion - as part of #107 I am updating and improving the story around mutable types and the commands API, and as it is looking now that will allow FsCheck to generate/shrink a broad range of mutable types out of the box (by finding ctors and applying a sequence of methods/properties on the instance). Pending that, it's better to postpone the discussion around ease of use.

[ploeh]

FWIW, I may have failed to understand either the motivations or the proposed change when I originally commented on this. If that's the case, I apologise.

If I understand it correctly, however, I think perhaps this proposed change would also address the original problem reported here: #334