soenneker / soenneker.utils.autobogus

The .NET Autogenerator
https://soenneker.com
MIT License
39 stars 4 forks source link

AutoFixture Extensability #263

Open jacob7395 opened 2 months ago

jacob7395 commented 2 months ago

Hey,

I have some thought about how to enhancement AutoFixture this is probably two issues but would like to get your opinion in one place befog creating some sub issues;

  1. Support integration with substitution libraries like NSubstitute
  2. Provide a generic way to allow to configure (set rules) for how AutoFaker generates specific types

Substitution Integration

Adding integration with substitution libraries shouldn't be too bad, the issue is many a lot of the components of AutoFixture are internal, or if not internal not using virtual to allow for extension f the required method. An option would be allowing the binder to be extended and making AutoFakerBinder.CreateInstance virtual. This would allow us to provide a specific binder that support the substitution library. Using the patten followed by the original library where the substitution bindings are provided as separate nuget packages to prevent unnecessary dependencies.

In addition to this a simple refactor to AutoFixture to use the interface IAutoFakerBinder (would require some extra methods to be pulled into it), could allow for more extendability. I will say that both changes option open up the door for people shooting themselves in the foot, but I don't see that as a reason to gatekeep the internals. This is a test library after all its hard to predict how people will want to use it so giving them the option to fiddle is my preference.

Fluent Type Building

While it is possible to configure specific types using the current AutoFaker and AutoFakerOverride, this approach is cumbersome and not intuitive. For example:

public record ChildModelDto(string StringValue);
public record BaseModelDto(string StringValue, ChildModelDto ChildModel);

public class BaseModelDtoFaker : AutoFaker<BaseModelDto>
{
    public BaseModelDtoFaker(AutoFakerConfig? autoFakerConfig = null) : base(autoFakerConfig)
    {
        RuleFor(d => d.StringValue, "BASEVALUE");
    }
}

public class BaseModelDtoFakerOverride(BaseModelDtoFaker faker) : AutoFakerOverride<BaseModelDto>
{
    public override bool Preinitialize => false;

    public override void Generate(AutoFakerOverrideContext context)
    {
        var rulesets = context.RuleSets is { } ? string.Join(',', context.RuleSets) : null;

        context.Instance = faker.Generate(rulesets);
    }
}

public class ChildModelDtoFaker : AutoFaker<ChildModelDto>
{
    public ChildModelDtoFaker(AutoFakerConfig? autoFakerConfig = null) : base(autoFakerConfig)
    {
        RuleFor(d => d.StringValue, "CHILDMODEL");
    }
}

public class ChildModelDtoOverride(ChildModelDtoFaker faker) : AutoFakerOverride<ChildModelDto>
{
    public override bool Preinitialize => false;

    public override void Generate(AutoFakerOverrideContext context)
    {
        var rulesets = context.RuleSets is { } ? string.Join(',', context.RuleSets) : null;

        context.Instance = faker.Generate(rulesets);
    }
}

public class ComplexModelGeneration
{
    [Fact]
    public void ConfigurePropertyModels()
    {
        var sharedConfig = new AutoFakerConfig()
        {
            Overrides = []
        };

        BaseModelDtoFaker baseFaker = new BaseModelDtoFaker(sharedConfig);
        sharedConfig.Overrides.Add(new BaseModelDtoFakerOverride(baseFaker));

        ChildModelDtoFaker childFaker = new ChildModelDtoFaker(sharedConfig);
        sharedConfig.Overrides.Add(new ChildModelDtoOverride(childFaker));

        var faker = new AutoFaker(sharedConfig);

        var b = faker.Generate<BaseModelDto>();
        var c1 = faker.Generate<ChildModelDto>();

        b.StringValue.Should().Be("BASEVALUE");
        b.ChildModel.StringValue.Should().Be("CHILDMODEL");

        c1.Should().BeEquivalentTo(b.ChildModel);
        c1.Should().NotBeSameAs(b.ChildModel);
    }
}

However, I don't believe the library was designed with this specific usage in mind. Therefore, I think it would be useful to provide a dedicated API to support this natively. I suggest introducing a fluent builder pattern that looks something like:

var faker = new AutoFaker(sharedConfig);

faker.RuleBuilder
    .RulesForType<BaseModelDto>(r => {
        r.RuleFor(b => b.StringValue, "BASEMODEL");
    })
    .RulesForType<ChildModelDto>(c => {
        c.RuleFor(d => d.StringValue, "CHILDMODEL")
    });

In the above I would expect the generation of BaseModelDto to use the rules defined in ChildModelDto. I don't think this should be supports out the gate but I wouldn't want to rule out something like this:

var faker = new AutoFaker(sharedConfig);

faker.RuleBuilder
    .RulesForType<BaseModelDto>(r => { //Where r would be a AutoFaker<BaseModelDto> or an interface that we can map to that type
        r.RuleFor(b => b.StringValue, "BASEMODEL");
        r.RulesForType<ChildModelDto>(c => {
            c.RuleFor(d => d.StringValue, "OVERRIDE")
        })
    })
    .RulesForType<ChildModelDto>(c => {
        c.RuleFor(d => d.StringValue, "CHILDMODEL")
    });

So the above could generate "CHILDMODEL" when just requesting a ChildModelDto but "OVERRIDE" when part of BaseModelDto. I think the override within the RulesForType<BaseModelDto> should be prioritized over the 'base' defection, maybe a priority like:

  1. Rules defined in child IFluentRuleTypeBuilder; RulesForType.RulesForType.RuleFor(d => d.StringValue
  2. Rules defined in base IFluentRuleTypeBuilder; RulesForType.RuleFor(d => d.StringValue
  3. Defined AutoFaker; these could be passed as to the RulesForType method
  4. Auto generated values;
interface IFluentRuleTypeBuilderBase
{
    public IFluentRuleTypeBuilder<T> RulesForType<T>(AutoFaker<T> baseFaker) where T : class;

    public IFluentRuleTypeBuilder<T> RulesForType<T>(AutoFaker<T> baseFaker, Func<IFluentRuleTypeBuilder<T>> configuration) where T : class;

    public IFluentRuleTypeBuilder<T> RulesForType<T>(Func<IFluentRuleTypeBuilder<T>> configuration);
}

interface IFluentRuleTypeBuilder<T> : IFluentRuleTypeBuilderBase
{
    // Implement the same interface as Faker
}

interface IFluentRuleBuilder : IFluentRuleTypeBuilderBase
{

}

If we open up AutoFaker in line with my first suggestions we could provide a separate package AutoFaker.FluentBuilder, that provide a new AutoFaker with this API specifically in mind. This would reduce the risk of breaking existing code, but may force workarounds or extensive overrides.


I would love to discuses both of these with you, how do you see the library being used? are these features you have though about supporting?

soenneker commented 2 months ago

Yes, I've known that the substitution libs have been needing to be done for a while... I should get on that soon. I can't remember completely, but I feel like there was an issue with simply making CreateInstance virtual because of underlying caching and some of the complexities with downstream manipulation.

I like the concept you have here with fluent rule generation. I'm not opposed to introducing something similar to what you have. I try my best to compose rather than inherit, thus I haven't had a huge need for complex rules personally.

soenneker commented 2 months ago

@jacob7395 I took a look at the substitution lib feature and indeed it is not as simple as making the method virtual because of the recursion protection and downstream caching.

Nevertheless, I believe I'm on a decent path and should be able to come up with something soon.

soenneker commented 2 months ago

@jacob7395 could you give one of these a try:

https://github.com/soenneker/soenneker.utils.autobogus.moq https://github.com/soenneker/soenneker.utils.autobogus.nsubstitute https://github.com/soenneker/soenneker.utils.autobogus.fakeiteasy

jacob7395 commented 2 months ago

Hey, missed you messaging yesterday. Will give them a try tomorrow morning 7am GMT.

jacob7395 commented 1 month ago

Had a busy day at work today, but I have downloaded the NSubstitute version since that is what I am familiar with. I have started making some tests to cover some 'realistic' situation. Found an issue I'll write out a few more then put up a PR for you to look over.

jacob7395 commented 1 month ago

So I put some test together for NSub, it is a good start it generates interface how I would expect but I think it needs to be able to fix values. For example in the test bellow the ShouldGenerateTheSameInterfaceClasses_WithNonTypedFaker fails, how I have used AutoFixture in the was to generate values interfaces that I am trying to test, configure them then generate the test class. This would be passed my frozen interface and I can use that the test the feature. They have a way to generate as frozen, but there api's are more build around generic generators that you use to build any type.

What do you think about having a method like AutoFaker.Freez<TType>(out TType Value) orAutoFaker.Generate(bool asFrozen)` where after the call future instance of that type would all be the same.

[Collection("Collection")]
public class NSubstituteAutoFakerBinderInterfaceTests
{
    public interface ITestNestedInterface
    {
        public ITestInterface NestedInterface();
    }

    public interface ITestInterface
    {
        public int GetValue();
    }

    public class TestInterfaceClass(ITestInterface i)
    {
        public ITestInterface Interface { get; } = i;
    }

    public record TestInterfaceRecord(ITestInterface Interface)
    {
        public ITestInterface Interface { get; } = Interface;
    }

    [Fact]
    public void ShouldGenerateInterfaceClasses_WithTypedFaker()
    {
        // arrange
        var faker = new AutoFaker<TestInterfaceClass>()
        {
                Binder = new NSubstituteAutoFakerBinder()
        };

        // act
        var instance = faker.Generate();

        // assert
        var act = () => instance.Interface.GetValue().Returns(10);
        act.Should().NotThrow();
    }        

    [Fact]
    public void ShouldGenerateTheSameInterfaceClasses_WithNonTypedFaker()
    {
        // arrange
        var faker = new AutoFaker()
        {
                Binder = new NSubstituteAutoFakerBinder()
        };
        ITestInterface i = faker.Generate<ITestInterface>();
        i.GetValue().Returns(10);

        // act
        var instance = faker.Generate<TestInterfaceClass>();

        // assert
        instance.Interface.GetValue().Should().Be(10);
    }

    [Fact]
    public void ShouldGenerateInterfaceRecords_WithTypedFaker()
    {
        // arrange
        var faker = new AutoFaker<TestInterfaceRecord>()
        {
                Binder = new NSubstituteAutoFakerBinder()
        };

        // act
        var instance = faker.Generate();

        // assert
        var act = () => instance.Interface.GetValue().Returns(10);
        act.Should().NotThrow();
    }
}
jacob7395 commented 1 month ago

Also the ShouldGenerateInterfaceRecords_WithTypedFaker fails, looks like AutoFaker is trying generate a type of Type, haven't looked into too much. I personally haven't used interfaces in records too much, if you want find a fix happy to just add some docs to the repo about not working with records, but it might be something obvious.

soenneker commented 1 month ago

Thanks for the tests. I'll check out that interface record issue.

This frozen functionality is definitely a new feature, the previous lib did not have this I believe. I'm hesitant to add a new method or bool on top level because I'm unsure how much use this would actually get.

Am I understanding this right that you want to get the same object (by reference, not a new instance with the same values) out if it has the same type signature? Why would that be preferable to storing the generation's result locally in a variable and reusing?

Could you create a new issue for that feature request? Please move the fluent thing out of this issue too as I fear it'll get lost.

jacob7395 commented 1 month ago

Makes sense to be a new feature created this issue in Nsub repo. Not sure if that is the best place to discuses it as it relates heavenly to AutoBogus but it's somewhere.

Will move the fluent part to a separate issues, and closed as I believe the changes you have already made addressed many of the protected members.

fteotini-remira commented 3 weeks ago

I'll check out that interface record issue.

Sadly this happens too using Moq. It tries to create a Type while populating a record

soenneker commented 3 weeks ago

@jacob7395 @fteotini-remira record generation with interface resolution should be fixed; it was due to the EqualityContract compiler generated property. Please give it a try.

fteotini-remira commented 2 weeks ago

I haven't the chance to try it. I'll come back to you when I'll have one