-
Notifications
You must be signed in to change notification settings - Fork 157
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
xUnit integration: Automatically discover Arbitraries from parameter types #103
Comments
Why don't you just derive from 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#. |
I don't derive from |
Also, by the way, in your sample you could have used the 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
... |
@fsoikin, nice tip; I din't know about that one. Thank you for sharing 👍 |
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? |
Attributing the module works, but it is verbose: I need to type in all types twice - once in
My production code (I will assume that by "actual" you mean "production") does not need to take dependency on FsCheck. All this, including Think of it this way: this change does not affect core functionality, does not disrupt anything outside of the [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 |
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:
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? |
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)
{
}
} |
@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:
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. |
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. 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 |
@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>();
} |
Ah, sorry, I think it's ok if you explicitly call a method to derive the |
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? 😧 |
@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. |
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! 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. |
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 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 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. 😜 |
But this is so much noise! Look: |
True, it would have been terser in F#. 😉 Often, I find that when a tool resists me, it prompts me to reconsider my current approach.
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. |
Some thoughts.
|
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 Now, responding to thoughts:
|
PS I updated the documentation at https://fscheck.github.io/FsCheck/TestData.html |
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.
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.
|
Re: point 1. There is already Command.Create to create a command without implementing a class. I totally forgot about that. |
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. |
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 |
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 toPropertyAttribute.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.
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.The text was updated successfully, but these errors were encountered: