Skip to content
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

Why does AF lack support for modifying elements in a collection? #361

Closed
phillip-haydon opened this issue Mar 26, 2015 · 17 comments
Closed
Labels

Comments

@phillip-haydon
Copy link

In regards to this SO question:

http://stackoverflow.com/questions/19286476/autofixture-how-do-i-assign-a-property-on-only-a-subrange-of-items-in-a-list

Why does AF lack support for this sort of thing. It makes it impossible (from what I can tell) to do this stuff in a customization, meaning the benefit of autofixture is completely lost.

@ploeh
Copy link
Member

ploeh commented Mar 26, 2015

As that answer explains: there are already language constructs available for such operations, so why should AutoFixture contain an ad hoc, informally-specified, bug-ridden, slow implementation of half of C# (or VB.NET, or F#)?

It makes it impossible (from what I can tell) to do this stuff in a customization, meaning the benefit of autofixture is completely lost.

This makes me curious, though. Why do you think that this is a requirement for making AutoFixture useful?

What are you trying to do with it?

FWIW, I've written thousands of tests using AutoFixture since it was created in early 2009, and while I've occasionally had the need to manipulate collection elements in a single test case here and there, I don't think I've ever had the need to do it in a Customization... so that's the reason I'm curious about your scenario.

@ploeh ploeh added the question label Mar 26, 2015
@phillip-haydon
Copy link
Author

AF has no support from documentation or blog posts, from what I can tell, to create a customization that modifies a subset of items in a collection so that calling CreateMany returns a list of type T which contain items that adhear to certain criteria.

Without of course, defeating the purpose of using AF to begin with and creating a method which just returns that list, which I of course now need to main.

What I want is to create 10 orders, with all data randomly mocked, like AF does, but say that certain orders contain certain criteria, thats set inside a customization, so it is reused across all tests using that customization.


That aside, now running into issues where the most basic 'Create' fails.

Class with a Base Abstract Class.

Call Create on the subclass.

Get error about:

Ploeh.AutoFixture.ObjectCreationException : AutoFixture was unable to create an instance of NodeBase, since it's an abstract class with a public constructor. Please consider changing the definition of NodeBase; while technically possible, an abstract class with a public constructor represents a design error. For more information please refer to: http://tinyurl.com/lg38t3g. If you are unable to modify the NodeBase class, you can customize AutoFixture by mapping the class to a concrete type. As an example, imagine that AbstractClassWithPublicConstructor is an abstract class with a public constructor, and that you can't change the definition of that class. In order to work around that issue, you can add a test-specific concrete class that derives from AbstractClassWithPublicConstructor:

I think I'll just flag using AF, already running into so many roadblocks with it for trivial things.

For what its worth, the code to create that error:

            var fixture = new Fixture();
            var order = fixture.Create<Order>();

Where Order is:

public class Order : NodeBase

@ploeh
Copy link
Member

ploeh commented Mar 27, 2015

What I want is to create 10 orders, with all data randomly mocked, like AF does, but say that certain orders contain certain criteria, thats set inside a customization, so it is reused across all tests using that customization.

This is crystal clear: I understand what you want to do, but I don't understand why you want to do it.

Your original question was, "Why does AF lack support for modifying elements in a collection?", and my original answer, both here, and on your recent Stack Overflow question is that the reason AutoFixture doesn't have that, is because you shouldn't need it.

I may be wrong; that's happened before.

That's the reason I ask you about your specific requirements, instead of your general requirements, because in general I've never had the need to do something like that, and I've been using AutoFixture for 6+ years...

I'd like to understand why you want to do something like that.


Can you provide more details about your specific issue? I've tried the following, but can't reproduce the error you're reporting.

public class NodeBase { }
public class Order : NodeBase { }

public class Tests
{
    [Fact]
    public void Repro()
    {
        var fixture = new Fixture();
        var order = fixture.Create<Order>(); // Doesn't throw
    }
}

@phillip-haydon
Copy link
Author

So we have a legacy system, where we have a God class called Node and then heaps of sub nodes that describe things in the system, that all relate back.

Delivery, Order, Version, Storyboard, File, etc.

60% of the system relies on Order/Version, based on a discriminator. A lot of code requires pulling this together at the same time.

So I have:

10 Nodes
[0] = ParentId: 0, Id: 1, Type: 1
[1] = ParentId: 1, Id = 2, Type: 2
[2] = ParentId: 1, Id = 3, Type: 2
[3] = ParentId: 1, Id = 4, Type: 2
[4] = ParentId: 0, Id: 5, Type: 1
[5] = ParentId: 5, Id = 6, Type: 2
[6] = ParentId: 5, Id = 7, Type: 2
[7] = ParentId: 5, Id = 8, Type: 2

Then the other 25 fields are randomly generated. So I want to avoid having to manually create lots of data, but have control over the relationships, and some specific settings that should be the same across the large majority of tests.

Now inside the tests I have no problem saying:

orders[0].Setting... = ... some specific test scenario

The issue is setting up all the initial data.

So I want to pull that into a customization and say right. For most of our tests which require this, its pre-configured to a valid scenario. And we override for test cases.


The second issue around the base-type and recursion. We support unlimited number of child nodes. So all nodes have a parent.

    [TestFixture]
    public class TestStuffs
    {
        [Test, Category(TestCategories.Unit)]
        public void Fail()
        {
            var fixture = new Fixture().Customize(new TestCustomization());

            var result = fixture.Create<TestClass>();
        }

        public abstract class TestClassBase
        {
            public IList<Node> Nodes { get; set; }
        }

        public class TestClass : TestClassBase
        {

        }

        public class Node
        {
            public TestClassBase Parent { get; set; }
        }

        public class TestCustomization : ICustomization
        {
            public void Customize(IFixture fixture)
            {
                fixture.Customizations.Add(new TypeRelay(typeof(TestClassBase), typeof(TestClass)));

                //How to prevent recursion???
                //fixture.Customize<TestClass>(x => x.With(f => f.Nodes, fixture.Build<Node>(n => n.Without(u => u.Parent))));
            }
        }
    }

This I cannot get working.

@ploeh
Copy link
Member

ploeh commented Mar 27, 2015

Thank you for sharing some specifics about your motivation for wanting to do what you ask about. That should be possible.

Here's a small proof of concept:

public class Node
{
    public int ParentId;
    public int Id;
    public int Type;
    public string Delivery;
    public string Order;
    public string Version;
}

public class NodeCustomization : ICustomization
{
    public void Customize(IFixture fixture)
    {
        fixture.Register(() => CreateNodes(fixture));
    }

    private static List<Node> CreateNodes(IFixture fixture)
    {
        var nodes = fixture.CreateMany<Node>(10).ToList();
        nodes[0].ParentId = 0; nodes[0].Id = 1; nodes[0].Type = 1;
        nodes[1].ParentId = 1; nodes[1].Id = 2; nodes[1].Type = 2;
        nodes[2].ParentId = 1; nodes[2].Id = 3; nodes[2].Type = 2;
        nodes[3].ParentId = 1; nodes[3].Id = 4; nodes[3].Type = 2;
        nodes[4].ParentId = 0; nodes[4].Id = 5; nodes[4].Type = 1;
        nodes[5].ParentId = 5; nodes[5].Id = 6; nodes[5].Type = 2;
        nodes[6].ParentId = 5; nodes[6].Id = 7; nodes[6].Type = 2;
        nodes[7].ParentId = 5; nodes[7].Id = 8; nodes[7].Type = 2;
        return nodes;
    }
}

public class Tests
{
    [Fact]
    public void CreateNodesUsingCustomizedFixture()
    {
        var fixture = new Fixture().Customize(new NodeCustomization());

        var nodes = fixture.Create<List<Node>>();

        Assert.Equal(0, nodes[0].ParentId);
        Assert.Equal(1, nodes[1].ParentId);
        Assert.Equal(1, nodes[2].ParentId);
        Assert.Equal(1, nodes[3].ParentId);
        Assert.Equal(0, nodes[4].ParentId);
        Assert.Equal(5, nodes[5].ParentId);
        Assert.Equal(5, nodes[6].ParentId);
        Assert.Equal(5, nodes[7].ParentId);
        Assert.NotEqual(default(string), nodes[0].Delivery);
        // etc.
    }
}

The NodeCustomization explicitly customises List<Node>, but you can repeat the Register call with different type arguments if you want to apply the same logic to other collection types. Here's an example for arrays:

fixture.Register<Node[]>(() => CreateNodes(fixture).ToArray());

With that out of the way, let's revisit the reason AutoFixture doesn't have a Domain-Specific Language for this sort of thing.

First, why should you be forced to learn a new, specialised API for this sort of thing, when you can already use your existing knowledge of C# (or VB.NET, or F#)? As you can see, the CreateNodes method uses existing language constructs to solve the problem.

Second, AutoFixture was originally created as a tool for Test-Driven Development. It's always a good sign if a piece of software can be picked up and used by other people in ways the original creators hadn't predicted, so I think it's cool if you would like to use it with a legacy system. However, the reason that AutoFixture doesn't have explicit support for addressing issues like the above is that it's an opinionated library: it tries hard to make it easy for you to do the right thing, and conversely, wants to make it hard for you to do the wrong things.

The design you sketch above looks like it could be improved. I do realise that you explained that it's a legacy system, and you probably can't address the design issues, which is also the reason I started this particular reply with a possible way to address the issue. However, it'd be much better not having that issue at all, and that's the reason AutoFixture doesn't have an API for that sort of thing.


The code you supplied does throw an exception, but not the exception you originally reported. Can we take that in another thread? Preferably on Stack Overflow.

@phillip-haydon
Copy link
Author

Thanks @ploeh. Ok so I missed Register completely. I tried Inject thinking that would do what you did above but it didn't work and that's when I threw in the towel and posted on here and SO.

I thought: "Ok, it's either gonna be in the API, or allow me modify it and store it" but I couldn't do either, I did look through blog posts and doco but never found what I was looking for.

Our application is about 8 years old, and we do TDD, but with so much legacy code we looked at AF to help facilitate some of the legacy code issues we have in unit tests. (we have around 10k unit tests, so there's lots of duplication in setup of objects)

I have no problems with AF being an opinionated library, but if I read in the doco what you said above i would be like "OH ok cool, no API for it, can still do it, just need to do it myself, no worries" but lacking both got me frustrated!


Can move to SO, will do on Monday.

@ploeh
Copy link
Member

ploeh commented Mar 27, 2015

FWIW, if I could do it all over again today, I would have put the documentation in an easier-to-find place, rather than scattered around in blog posts etc...

@moodmosaic
Copy link
Member

Indeed, I think AtomEventStore's wiki is a good example.

@ploeh
Copy link
Member

ploeh commented Mar 27, 2015

@moodmosaic, well, it's the best I could come up with, but it's coupled to GitHub, so that could become an issue...

Also, there are issues with GitHub wikis that I'm not completely happy with.

@moodmosaic
Copy link
Member

@ploeh A good alternative can be Read the Docs which has PDF Generation, full-text search, and more.

@phillip-haydon
Copy link
Author

@ploeh what are you not happy with. I think Nancy wiki is pretty decent. And while lacking I think I did a decent job with Snow. The wiki is easier to work with when you pull the repo down. Then you can add images.

@ploeh
Copy link
Member

ploeh commented Mar 27, 2015

@phillip-haydon, TBH, I can't remember if there was anything besides this, but I dislike that the Pages list on the right is a flat list without any sort of structure. See e.g. https://github.com/GreanTech/AtomEventStore/wiki

@phillip-haydon
Copy link
Author

Is not flat :D

https://github.com/nancyfx/nancy/wiki

@adamchester
Copy link
Member

That's a "side bar" though (which looks nice!)

On 28 Mar 2015, at 10:34 am, Phillip Haydon notifications@github.com wrote:

Is not flat :D

https://github.com/nancyfx/nancy/wiki


Reply to this email directly or view it on GitHub.

@ploeh
Copy link
Member

ploeh commented Mar 28, 2015

That does look nice - I wasn't aware of that feature. Thanks for the tip 👍

@ploeh
Copy link
Member

ploeh commented Apr 10, 2015

@phillip-haydon, did you get an answer to your original question?

If not, how can we help you?

If so, can we close the issue?

@phillip-haydon
Copy link
Author

Sorry, I got pulled onto some other stuff and haven't had a chance to go back to the original issue. I'll raise an issue on Github when I get to it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants