Fill Readonly Collection Properties Behavior

[charles-salmon]

This PR addresses #341.

I've implemented this using a behavior, per the recommendation of @zvirja. However, I have a slight concern with this, in that I do not believe it is possible to determine whether or not a given property has been marked with an OmitSpecimen signal from the context of a behavior (e.g. the addition of this behavior will still populate readonly collection properties, if OmitAutoProperties has been called or if Without has been used to explicitly exclude a particular property). I'm prepared to make any changes necessary, following on from feedback in this regard.

Is anyone potentially able to take a look at this? @ploeh @moodmosaic @adamchester @ecampidoglio @zvirja @sgryt

This is my first contribution to AutoFixture, so please feel free to be critical.

[WestDiscGolf]

We were just having a discussion about this at work and were excited to see this PR. Hope it makes the next version 👍

[aivascu]

@charles-salmon first of all, thank you for the contribution! This is a feature expected by many.

Overall the PR looks good in terms of implemented functionality. 👍

I left some comments on some code-convention and design choices. My hope is you will not get scared away by them. 🙂 I am open for discussion around any comment.
If you need it I can make a commit to better exemplify the code changes I've requested.

I still need to investigate on my own the concerns you have raised and how to mitigate them, so I'll get back with more info when I find anything.

[charles-salmon]

@aivascu Thank you very much for taking a look at this. I sincerely apologize that it took me several months to find the time to action the feedback that you'd given, but I finally managed to block out a chunk of time to get to this, and have responded to all of the issues that you've raised.

Also, in response to:

I've implemented this using a behavior, per the recommendation of @zvirja. However, I have a slight concern with this, in that I do not believe it is possible to determine whether or not a given property has been marked with an OmitSpecimen signal from the context of a behavior (e.g. the addition of this behavior will still populate readonly collection properties, if OmitAutoProperties has been called or if Without has been used to explicitly exclude a particular property). I'm prepared to make any changes necessary, following on from feedback in this regard.

I took a closer look at this. When a property is explicitly excluded using the Without method on a readonly property, an appropriate exception is thrown, indicating that the property is not writable. This is thus much lesser of a concern that I had first thought.

Let me know if you're happy with how this has shaped up! 😄

[aivascu]

@charles-salmon amazing work! The PR looks good aside from a couple moments, most important of which is whether the behavior should be ON by default.
@ecampidoglio @zvirja I'd like to know what you think. Should we enable this by default?

[charles-salmon]

@aivascu Thanks once again for the review! I have actioned the feedback that you've given, and agree that this should perhaps not be a default behavior.

[aivascu]

LGTM 👍

[pentarex]

I am getting AmbiguosMatchFound Exception here, the Collection that I am using is RepeatedField from protobuf, Not sure if this can happen anywhere else, but maybe worth checking in.
I have solved it like:

public IEnumerable<IMethod> SelectMethods()
        {
            if(Owner == null) return Array.Empty<IMethod>();
            var methods = this.Owner.GetType().GetMethods();
            foreach (var methodInfo in methods)
            {
                if (methodInfo.Name == MethodName && methodInfo.GetParameters().Length == 1)
                {
                    return new IMethod[]
                    {
                        new InstanceMethod(methodInfo, this.Owner)
                    };
                }
            }
            
            return Array.Empty<IMethod>();
        }

Not sure if ideal, but desperate time - require desperate measures :)