Add auto-generator #12

cmeeren · 2017-11-13T11:52:10Z

As discussed in hedgehogqa/fsharp-hedgehog#133. We can continue developing it further in this PR.

Please review it carefully and pay special attention to the questions at the bottom.

Usage in its initial form:

gen {
 // Using default generators (type inferred)
 let! x = Gen.auto()

 // Overriding generators (type inferred)
 let! x = Gen.auto'({Gen.defaults with Char = Gen.alphaNum})

  // You can also explicitly specify the type to generate:
  let! x = Gen.auto<MyType>()
  let! x = Gen.auto'<MyType>(...)
}

Point for consideration: I think it might be better to place auto in a class named Gen in the Hedgehog namespace, and have optional arguments for all the generators. That way, we get the following benefits:

Only one function (auto) instead of two (auto and auto'), because auto' can just be an overload of auto. However, see next point.
Instead of overloading, we can use optional arguments for all the generators. Callers of auto can then just specify the arguments corresponding to the generators they want to override. This gets rid of the AutoGenConfig type and the defaults value (at least publicly) which didn't really add any inherent value.
Considering the previous point, it becomes much more C#-friendly. With the current implementation I think we'd have to mark AutoGenConfig with [<CLIMutable>] and/or [<Struct>], and C# users would still have to assign default to a variable and modify its properties before using it.
It even gets rid of some F# boilerplate: Gen.auto(char=Gen.alphaNum) instead of Gen.auto({Gen.defaults with Char = Gen.alphaNum}).

In short, I think the above speaks fairly well for wrapping this in a class.

cmeeren · 2017-11-13T12:39:35Z

The only problem with my suggestion is that I have to add [<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>] to the Gen module since both it and the Gen type lives in the Hedgehog namespace. I don't know exactly what this entails - does it mean that C# users have to use GenModule when accessing functions in the Gen module?

cmeeren · 2017-11-13T12:42:50Z

I've created a branch on my repo called gen-auto-alt that shows the alternative impementation. Here's the commit that shows the changes between this and the alternate version.

moodmosaic · 2017-11-13T16:41:56Z

Both approaches are interesting, while with a quick look I think I lean towards the alternative one, I need to look at both closer and compare them from the C# point of view too. 🚀 🎉

1. Use explicit qualified access to Gen module to make it clear which function is in Hedgehog and which one is in Hedgehog.Experimental. 2. Because of 1., use a namespace 'Hedgehog' and a module 'Gen'. 3. Indent body of Computation Expressions with 4 spaces. 4. open System and remove all occurences of 'System.'. 5. Make 'dateInterval' consistent with other functions, where we have the bounded value starting after the '=' in a new line. We usually do this for functions with lines more than 80 chars. 6. Add a space after each function parameters, even if it's the empty tuple, for example: foo() becomes foo (). 7. Format test code according to this heuristic: http://blog.ploeh.dk/2013/06/24/a-heuristic-for-formatting-code-according-to-the-aaa-pattern/ 8. Start comments with capital letter and end with period, for example: '// this is foo bar' becomes '// This is foo bar.'. 9. Remove redundant parentheses.

moodmosaic · 2017-11-14T14:28:35Z

Both approaches look interesting, and both have their pros and cons. I think we should focus on how we can easily consume the auto stuff from C#, because we now support LINQ queries in the F# Hedgehog.

In the tests I did in both branches, I used these types:

type Actions =
    { Tag      : string
      Contents : obj }
    override this.ToString () =
        sprintf
            "%s %A"
            this.Tag
            this.Contents

type Type =
    { Tag      : string
      Contents : Actions }
    override this.ToString () =
        sprintf
            "%s %A"
            this.Tag
            this.Contents

type Transactions =
    { Timestamp : int64
      Signature : string
      To        : string
      Origin    : string
      Header    : Type }
    override this.ToString () =
        sprintf
            "%A %s %s %s %A"
            this.Timestamp
            this.Signature
            this.To
            this.Origin
            this.Header

type TransactionParams (txs : Transactions) =
    member val Method = "Transaction" with get, set
    member val Params = txs           with get, set
    override this.ToString () =
        sprintf
            "%s %A"
            this.Method
            this.Params

The first issue was the C# compiler complained, since we now have 2 Gen types; one in Hedgehog.dll and one in Hedgehog.Experimental.dll.

In order to make the following code to compile, I had to rename this project's Gen into GenX—or we can find a better name—as we can't modify Hedgehog itself (it'll be a breaking change):

using Hedgehog;

class Program
{
    static void Main(string[] args)
    {
        var a = GenX.Auto<TransactionParams>();
        Gen.printSample(a);

        var @default =
            GenX.defaults;
        var gnConfig =
            new AutoGenConfig(
                  @byte          : @default.Byte
                , @int16         : @default.Int16
                , @int           : @default.Int
                , @int64         : @default.Int64
                , @double        : @default.Double
                , @decimal       : @default.Decimal
                , @bool          : @default.Bool
                , guid           : @default.Guid
                , @char          : Gen.ascii
                , @string        : @default.String
                , dateTime       : @default.DateTime
                , dateTimeOffset : @default.DateTimeOffset
                , range          : @default.Range
                );
        
        var b = GenX.AutoWith<TransactionParams>(gnConfig);
        Gen.printSample(b);
    }
}

Note that GenX.Auto and GenX.AutoWith are generated by the F# compiler, by decorating the F# functions with [<CompiledName ("Auto")>] and [<CompiledName ("AutoWith")>] respectively.

In F#, things are easier thanks to F#'s copy-and-update feature:

open Hedgehog

[<EntryPoint>]
let main argv =
    Gen.printSample <| GenX.auto'<TransactionParams> { GenX.defaults with Char = Gen.ascii }

`gen-auto-alt` branch

open Hedgehog

[<EntryPoint>]
let main argv =
    Gen.printSample <| Gen.auto<TransactionParams> (char = Gen.ascii)

In this branch, it's required to decorate each one of the auto parameters with the [<Optional>] attribute, as described here.

static member auto<'T> ([<Optional>] ?byte, [<Optional>] ?int16, [<Optional>] ?int [..]

And then, from C# we can do something like this:

using Hedgehog;

class Program
{
    static void Main(string[] args)
    {
        var a = GenX.auto<TransactionParams>();
        Gen.printSample(a);

        var b =
            GenX.auto<TransactionParams>(
                @char: FSharpOption<Gen<Char>>.Some(Gen.alphaNum));
        Gen.printSample(b);
    }
}

So, it boils down to

// This pull request.
class Program
{
    static void Main(string[] args)
    {
        var a = GenX.Auto<TransactionParams>();
        Gen.printSample(a);

        var @default =
            GenX.defaults;
        var gnConfig =
            new AutoGenConfig(
                  @byte          : @default.Byte
                , @int16         : @default.Int16
                , @int           : @default.Int
                , @int64         : @default.Int64
                , @double        : @default.Double
                , @decimal       : @default.Decimal
                , @bool          : @default.Bool
                , guid           : @default.Guid
                , @char          : Gen.ascii
                , @string        : @default.String
                , dateTime       : @default.DateTime
                , dateTimeOffset : @default.DateTimeOffset
                , range          : @default.Range
                );
        
        var b = GenX.AutoWith<TransactionParams>(gnConfig);
        Gen.printSample(b);
    }
}

vs

// That gen-auto-alt branch.
class Program
{
    static void Main(string[] args)
    {
        var a = GenX.auto<TransactionParams>();
        Gen.printSample(a);

        var b =
            GenX.auto<TransactionParams>(
                @char: FSharpOption<Gen<Char>>.Some(Gen.alphaNum));
        Gen.printSample(b);
    }
}

If we can get rid of that pesky

@char: FSharpOption<Gen<Char>>.Some(Gen.alphaNum));

then I think we should consider the gen-auto-alt branch, otherwise this pull request is the way to go.

/cc @jystic

Housekeeping

cmeeren · 2017-11-14T14:43:58Z

What does all the @mean in the C# code?

And could we use CompiledName or some other attribute to use GenX only in C# and Gen in F#?

moodmosaic · 2017-11-14T15:42:14Z

What does all the @mean in the C# code?

It's required if a word is reserved, like byte, string and other reserved words in C#.

And could we use CompiledName or some other attribute to use GenX only in C# and Gen in F#?

No, I don't think that's easy, at least off the top of my head.

cmeeren · 2017-11-14T17:27:57Z

No, I don't think that's easy, at least off the top of my head.

Might I then suggest we add an auto function to the Gen module that simply calls GenX.auto()? That way, one can just use Gen.auto for all simple purposes, and then use GenX.auto only when one needs to override the default generators.

That is, of course, if we go for the alternative implementation (not this PR).

moodmosaic · 2017-11-15T07:44:33Z

The problem (if we can call it like this way) is the C# compiler; it won't allow two Gen types in different assemblies.

cmeeren · 2017-11-15T07:48:38Z

The problem (if we can call it like this way) is the C# compiler; it won't allow two Gen types in different assemblies.

Perhaps I didn't make myself sufficiently clear in my previous comment. I am talking about just having let auto = GenX.auto in the Gen module, so that users who just want simple auto-generation (no overriding) don't have to use GenX.

moodmosaic · 2017-11-15T07:52:14Z

No, you are actually pretty precise 👍 It's just that Gen module isn't going to happen here. We have to rename it to GenX... Yes, we could keep around both, one for C# and for F#, but it doesn't feel 'normal'(?)

cmeeren · 2017-11-15T07:54:48Z

Oh, I see what you mean - Hedgehog.Experimental can't contain a Gen module at all if it's to be used from C#. Well then. In that case, i think GenX is a nice name. But could we have our cake and eat it too, i.e., could we somehow make Gen usable from F# and have GenX from C#? If that is even desirable? In any case, this seems like a discussion that belongs in a separate issue/PR.

cmeeren · 2017-11-15T07:55:35Z

Back to the auto-generator: It would be great if the user could also specify overrides for any complex type they desire. For example, say Customer is a type that has a field of type Customer list (sub-customers of the current customer). This is recursive and (currently) needs a specifically designed generator to avoid stack overflow. Say then that I want to generate an Order type which, among other things, contains a Customer, then I'd want to just say auto<Order>(...) and somehow specify that I want Customer objects to be generated using my specific generator, and otherwise the defaults are fine.

I'm not sure how to do this though, or if it is easy/feasible/possible.

moodmosaic · 2017-11-15T08:03:54Z

But could we have our cake and eat it too, i.e., could we somehow make Gen usable from F# and have GenX from C#?

I'd say; let's go for GenX everywhere now in this PR, and we can tackle this in detail on some other PR, if you agree?

It would be great if the user could also specify overrides for any complex type they desire.

That's a good point; currently with the auto you propose we can only override the primitive generators. I don't know how we can model this, given that we don't have (Haskell) type classes in .NET...

Perhaps, we could pass some dictionary of type 'a -> Gen<'a> and then the auto can look up on that dictionary(?)

First things first, do you agree we should rule out the gen-auto-alt branch? It's neat, but requires the auto parameters to be decorated with the [<Optional>] attribute, as described here.

static member auto<'T> ([<Optional>] ?byte, [<Optional>] ?int16, [<Optional>] ?int [..]

And then, the usage is a bit clunky in C#:

class Program
{
    static void Main(string[] args)
    {
        var a = GenX.auto<TransactionParams>();
        Gen.printSample(a);

        var b =
            GenX.auto<TransactionParams>(
                @char: FSharpOption<Gen<Char>>.Some(Gen.alphaNum));
        Gen.printSample(b);
    }
}

cmeeren · 2017-11-15T08:16:03Z

I'd say; let's go for GenX everywhere now in this PR, and we can tackle this in detail on some other PR, if you agree?

Sure.

Perhaps, we could pass some dictionary of type 'a -> Gen<'a> and then the auto can look up on that dictionary(?)

Not sure how to implement that. Could you sketch out something for me? Just an example so I have something to go on (though I think in this case the distance between example and complete implementation is fairly short).

First things first, do you agree we should rule out the gen-auto-alt branch?

I agree that it should be invisible to C# users that this library is written in F#, so FSharpOption is a no-go, but this PR seems perhaps even more clunky, requiring C# users to specify the whole configuration when changing just one generator. Is there any way to make it work nicely for both F# and C# users?

moodmosaic · 2017-11-15T08:20:00Z

So, if you agree

Let's change Gen to GenX in this pull request.
We can tackle 'a -> Gen<'a> separately, as I need to find some time to sketch something for it.
Probably it's fine for now for the C# users to have something like that gnConfig where they can copy and paste around.

cmeeren · 2017-11-15T08:23:45Z

Probably it's fine for now for the C# users to have something like that gnConfig where they can copy and paste around.

Couldn't we solve this by marking the AutoGenConfig type with CLIMutable and Struct? If I'm not mistaken, then C# users could assign GenX.defaults to a local variable in their tests and just set the properties they want to override. Am I right?

moodmosaic · 2017-11-15T08:37:20Z

Oh, yes, we can decorate AutoGenConfig with just [<CLIMutable>] and the C# boilerplate will go away
🎉

cmeeren · 2017-11-15T08:38:56Z

Won't we have to use [<Struct>] also to avoid users modifying the actual Hedgehog.GenX.defaults value?

moodmosaic · 2017-11-15T08:44:29Z

That, I don't know off the top of my head (I only have a Haskell compiler in front of me right now)

However, I assume this isn't available in F# 4.0; I think they've made it available in F# 4.1...

cmeeren · 2017-11-15T08:54:32Z

I can use Struct in the project, so I assume it's available.

What I am talking about is this (C# code):

var gens = GenX.defaults;
gens.Char = Gen.alphaNum;

Without Struct, I think the last line above will modify the actual GenX.defaults object, since it's passed by reference. This will mess with other tests. Whereas with Struct, a copy is created on the first line which is then modified on the second line (in fact, if I remember correctly how structs work, the second line also creates a copy and sets it to gens, but that's irrelevant in this context).

moodmosaic · 2017-11-15T08:59:41Z

Yes, sounds good if we can do it. Perhaps we can have F# Hedgehog Experimental even target F# 4.1 if that will allow us to use the [<Struct>]...

cmeeren · 2017-11-15T10:32:14Z

Made requested changes and rebased on current master. See if I've done it correctly.

cmeeren · 2017-11-15T10:35:13Z

Oh fudge. For some reason VS committed loads if irrelevant stuff in the merge commit. I think I'll stick to the command line hereafter.

Let me know how to proceed. I can check out the current master and make a new PR of you want.

moodmosaic · 2017-11-15T12:04:24Z

No worries

Could you please remove that Paket.Restore.targets file for now? We can add it later, separately, as part of the paket auto-restore functionality, which is nice to have 😃
Also, can you add this to .gitgnore ?
```
# Visual Studio 2015 cache/options directory
.vs/
```
This will remove those files that are showing up inside .vs.

cmeeren · 2017-11-15T13:12:56Z

Looking at the diff, there seems to be several other strange changes I can't remember making, which I expect was part of #13. I suppose the rebasing got botched somewhere. Should I just create a new "clean" PR with the current implementation?

cmeeren · 2017-11-15T13:14:08Z

We could then also factor out the GenX transformation into its own PR, which makes it a bit cleaner. Those changes don't really belong here.

moodmosaic · 2017-11-15T13:15:59Z

A clean PR (in terms of git commit history) is always welcome, if it's OK for you 👍

cmeeren · 2017-11-15T13:19:14Z

Yes, I certainly prefer that.

cmeeren · 2017-11-15T13:33:54Z

Talking about having a clean history; have you seen this? https://github.com/blog/2141-squash-your-commits?

moodmosaic · 2017-11-15T14:05:22Z

Yes :)

Add auto-generator

a96ce12

Merge pull request #13 from hedgehogqa/topic/housekeeping

88106d0

Housekeeping

cmeeren added 6 commits November 15, 2017 11:19

Add CLIMutable and Struct to make AutoGenConfig easier to use from C#

758bb23

Rename main module to GenX

57b409c

Add auto-generator

10bd4c3

Add CLIMutable and Struct to make AutoGenConfig easier to use from C#

b52c847

Rename main module to GenX

9a7baab

Merge

74d725e

remove accidentally added files, add .vs/ to gitignore

075fc29

cmeeren closed this Nov 15, 2017

cmeeren mentioned this pull request Nov 15, 2017

add auto-generator #17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add auto-generator #12

Add auto-generator #12

cmeeren commented Nov 13, 2017 •

edited

Loading

cmeeren commented Nov 13, 2017

cmeeren commented Nov 13, 2017

moodmosaic commented Nov 13, 2017

moodmosaic commented Nov 14, 2017 •

edited

Loading

cmeeren commented Nov 14, 2017

moodmosaic commented Nov 14, 2017 •

edited

Loading

cmeeren commented Nov 14, 2017 •

edited

Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

Add auto-generator #12

Add auto-generator #12

Conversation

cmeeren commented Nov 13, 2017 • edited Loading

cmeeren commented Nov 13, 2017

cmeeren commented Nov 13, 2017

moodmosaic commented Nov 13, 2017

moodmosaic commented Nov 14, 2017 • edited Loading

gen-auto-alt branch

cmeeren commented Nov 14, 2017

moodmosaic commented Nov 14, 2017 • edited Loading

cmeeren commented Nov 14, 2017 • edited Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017 • edited Loading

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 15, 2017

cmeeren commented Nov 15, 2017

moodmosaic commented Nov 15, 2017

cmeeren commented Nov 13, 2017 •

edited

Loading

moodmosaic commented Nov 14, 2017 •

edited

Loading

`gen-auto-alt` branch

moodmosaic commented Nov 14, 2017 •

edited

Loading

cmeeren commented Nov 14, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading

cmeeren commented Nov 15, 2017 •

edited

Loading