Creating MailMessage doesn't work

[tiesmaster]

Similar to #441, creating a MailMessage doesn't work without customizing an instance of AutoFixture, as shown by this test:

    [Fact]
    public void FixtureCanCreateMailMessage()
    {
        var fixture = new Fixture();
        var actual = fixture.Create<MailMessage>();
        Assert.NotNull(actual);
    }

Which throws TargetInvocationException.

This is because the encoding properties (BodyEncoding, HeadersEncoding, and SubjectEncoding) require an System.Text.Encoding instance, and it appears that AutoFixture is not able to create those as well.

Injecting something like Encoding.UTF8 solves the problem. Maybe the same approach could be taken like #427, by adding a Utf8EncodingGenerator.

[ploeh]

It's true that AutoFixture can't create System.Net.Mail.MailMessage objects out of the box.

It's quite trivial to make it:

var actual = fixture.Build<MailMessage>().OmitAutoProperties().Create();

or, if you want a Customization:

fixture.Customize<MailMessage>(c => c.OmitAutoProperties());

Granted, in both cases, the MailMessage object will be quite empty...

Should AutoFixture be able to create MailMessage out of the box?

I'm torn:

Advantages

AutoFixture aims for the pit of success. I honestly have my doubts whether we've succeeded being there. Mostly, I think I've failed in pulling AutoFixture in that direction, but AutoFixture still works fairly well in many cases. It's certainly not unsuccessful...

Still, according to that goal, it's important that it just works, which it patently doesn't for MailMessage. According to this principle, therefore, AutoFixture ought to be able to generate MailMessage objects out of the box.

Disadvantages

The .NET Base Class Library (BCL) has thousands of types. It was never my goal that AutoFixture should be able to create all of them. For one, it'd be a Sisyphean task; also, it'd be an arms race, because new types are added with each new release of .NET.

Instead, I always envisioned giving AutoFixture a good set of heuristics for figuring out how to create objects and other values (I'll get back to this later), knowing full well that it wouldn't address 100% of all types.

To that, I added specific strategies for well-known primitives, such as Int32, Int64, Decimal, Double, String, Bool, Guid, DateTime, etc.

Over time, we've added a few more of those.

My major problem with adding explicit support for MailMessage is that it looks a little to me like a 'legacy' BCL type.

One of the issues (perhaps the most important one) we have for AutoFixture 4 is #404 (Support for CoreCLR). I don't know if MailMessage is going to be in CoreCLR or not, but I did a quick scan of the repo, and I couldn't find anything named Mail.

We already have enough features the we need to filter out in one way or another in order to be able to make AutoFixture run on CoreCLR, but I'm not sure adding more is a wise decision at the moment...

New heuristics

If it's only BodyEncoding, HeadersEncoding, and SubjectEncoding that constitute the problem with creating MailMessage objects, though, there may be another way.

All three are properties of the type Encoding, which is an abstract class.

What's special about this abstract class, though, is that it has static properties of its own type; e.g.

• Encoding.ASCII (which name violates the .NET design guidelines, BTW...)
• Encoding.Utf8
• Encoding.Utf32
• etc...

AutoFixture already has FactoryMethodQuery, which implements a heuristic that looks for members similar to these properties. The only difference is that it looks for methods, but not properties and fields.

Would it make sense to add a heuristic to AutoFixture that does the same for static properties and fields? Could it break existing code?

---

I'm honestly not sure about the 'right' direction to take here, so I'd like to solicit opinions on the matter, particularly, but not exclusively, from the AutoFixture core members @adamchester, @ecampidoglio, and @moodmosaic.

[ploeh]

BTW, @tiesmaster, thank you for bringing this up 👍

As you can see, it's an interesting enough question that I got a little carried away 😳

[moodmosaic]

IMHO, it's not hard to create System.Net.Mail.MailMessage objects, as shown in the two examples provided by @ploeh.

Granted, it takes an additional call to the Fixture instance, but in most (if not all) scenarios, the Fixture instance is customized...

And while this results to an empty object:

fixture.Customize<MailMessage>(c => c.OmitAutoProperties());

This results to a filled one:

var fixture = new Fixture();
fixture
    .Customize<MailMessage>(c => c
        .FromFactory(() =>
            new MailMessage(
                from   : fixture.Create<MailAddress>().ToString(),
                to     : fixture.Create<MailAddress>().ToString(),
                subject: fixture.Create<string>(),
                body   : fixture.Create<string>()))
        .OmitAutoProperties());

var actual = fixture.Create<MailMessage>();

On a side note: The System.Net.Mail.MailMessage seems to defined in the System.dll which looks like it is part of the PCL, though it doesn't seem to show up in the CoreCRL API reference page.

---

Based on @ploeh's comment, and the above, I probably wouldn't add a built-in generator for this. It is definitely an interesting question though, so thank you @tiesmaster for asking 👍

[tiesmaster]

@ploeh and @moodmosaic Thanks for the elaborate discussion, and I agree it's a difficult call to make, especially with CoreCLR coming up.

Having the creation of MailMessage out of the box, would be very nice and that’s what you very often get with it. I don’t really expect AutoFixture to be able to build every BCL type and not having MailMessage being constructed wouldn’t be the end for me. You’re right, it’s quite old, and not in CoreCLR yet. There is an issue for that though, but it may take a while before it's back.

---

However, the lengthy post also resulted in some more reflection on my side, and I realized that this isn’t really about MailMessage, but rather about objects with a member of type Encoding. So very similar to the other issue for objects with CultureInfo properties, so it might make sense to introduce support for that. That one is definitely in CoreCLR, so that probably won't interfere with migrating to that. Then again, this also raises the question why there has never been an issue for fixture.Create<Encoding>()...

Next, it also triggered me to ask "why did I stumble upon this?", and "why did I raise this issue?". The problem was that the object creation failed, and the exception message, and stack trace was rather poor. Although, it was clear that it wasn't able to create this type of object, but not why, and how to solve this.

So I tried to find out why it was failing, found out that it was because of properties with type of Encoding, and raised an issue with a solution. I did that because I wanted to improve AutoFixture, and make it very easy for other developers to create instances of this type. However, I totally missed the point that it was difficult to fix this, and that the feedback loop might needs some improvement here. A good exception message would have been much appreciated ;) Shall I create a different issue for that?

---

Regarding factory properties and fields, I do like that approach. However, I'd expect that it might lead to some really modified behavior for AutoFixture, in some situations. So that wouldn't be my first approach, if we still want to fix this.

[moodmosaic]

[..] it might lead to some really modified behavior for AutoFixture, in some situations.

Given that in most (if not all) scenarios the Fixture instance is customized, what do you mean by that?

[tiesmaster]

AutoFixture already has FactoryMethodQuery, which implements a heuristic that looks for members similar to these properties. The only difference is that it looks for methods, but not properties and fields.

@ploeh Playing around a little bit with this, I found out that FactoryMethodQuery is already accessing properties as factory methods: when you do fixture.Create<Encoding>(), AutoFixture will throw as indicated in the gist, however, it's doing that because it's auto-filling all of the properties of the Encoding instance that it created. You may ask, which Encoding instance, since that class is abstract. Well, it appears that FactoryMethodQuery finds get_ASCII() as factory method, and uses that to create a new specimen for that type. FactoryMethodQuery gets that class, since it's using reflection to find factory methods, and on that level properties are just methods.

So the need for a "PropertyQuery" or something like that, won't be necessary, since AutoFixture already does that ;)

---

Given that in most (if not all) scenarios the Fixture instance is customized, what do you mean by that?

@moodmosaic I don't know exactly what you mean by "[...] most (if not all) scenarios the Fixture instance is customized", since in a lot of situations for me at work, the Fixture instance is not customized. However, I don't think this is still really relevant, because, as indicated above, AutoFixture already does that ;)

[adamchester]

It looks like the problem is that a property or field setter (Encoding) throws an exception. Wouldn't the best solution be to have a much better exception message when this happens?

[adamchester]

I would not be in favour of adding a default configuration for MailMessage or Encoding or any similar types, only because I feel like it wouldn't be so common to create instances of these types for tests. I'm not basing this on any actual data; it's just a gut feeling.

Also, as the user of AutoFixture, trying to create robust tests, shouldn't you be aware of skipped properties in instances you are using? How do we make you aware the we automatically skipped some properties of MailMessage (or similar) instances?

[ploeh]

@tiesmaster, thank you for challenging my assumptions.

Digging a little further, it turns out that creation fails when AutoFixture attempts to populate an ASCIIEncoding object's properties. Apparently, the object is read-only.

Although I agree with @adamchester that a better exception message wouldn't hurt, the information is actually already there. If you read the entire exception message, you'll see this:

---- System.InvalidOperationException : Instance is read-only.
[...]
----- Inner Stack Trace -----
at System.Text.Encoding.set_EncoderFallback(EncoderFallback value)

What we could consider would be to add a feature to AutoPropertiesCommand that first looks for an IsReadOnly boolean property, and if the value of that property is false, it'll omit filling out auto-properties.

I'm not saying that this will allow AutoFixture to create instances of MailMessage, as there may also be other issues, but it may be worth investigating.

[adamchester]

Always looking for an IsReadOnly property could have some nasty impacts; it sounds dangerous (unless it was limited to certain known types where IsReadOnly has that exact meaning).

I think including the requestPath and a nice explanation of how the user might be able to fix the problem would be a good start?

For example, since the property setter threw an exception, we can explain the few options available: customise with a factory, ignore/without, provide a default value, etc?

Regards,
Adam Chester

On 31 Dec 2015, at 22:39, Mark Seemann notifications@github.com wrote:

@tiesmaster, thank you for challenging my assumptions.

Digging a little further, it turns out that creation fails when AutoFixture attempts to populate an ASCIIEncoding object's properties. Apparently, the object is read-only.

Although I agree with @adamchester that a better exception message wouldn't hurt, the information is actually already there. If you read the entire exception message, you'll see this:

---- System.InvalidOperationException : Instance is read-only.
[...]
----- Inner Stack Trace -----
at System.Text.Encoding.set_EncoderFallback(EncoderFallback value)
What we could consider would be to add a feature to AutoPropertiesCommand that first looks for an IsReadOnly boolean property, and if the value of that property is false, it'll omit filling out auto-properties.

I'm not saying that this will allow AutoFixture to create instances of MailMessage, as there may also be other issues, but it may be worth investigating.

—
Reply to this email directly or view it on GitHub.

[Pvlerick]

I actually encountered the same issue a few months ago, and I solved it the simplest way possible by regitering the Encoding type

fixture.Register(() => Encoding.UTF8);

The behaviour described previously is what I understood at the time. The fundamental issue is that System.Text.Encoding is by default marked as read-only (even subclasses can't override this), the only case seems to be when you Clone() it.

Would it be a sin to consider adding exceptions for well known types of the BCL that pose problems? The choice of the returned encoding in this case would be arbitrary, of course, but I think average users would be happier if things just worked for BCL types.

[adamchester]

It would not be the end of the world to add exceptions for common BCL types. The problem is the magic behind it remains hidden, I'm not sure what the right approach is here; needs a @ploeh decision:)

[Pvlerick]

I just tried to create a System.Net.IPAddress and as I feared, this also throws an exception:

Unhandled Exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.Net.Sockets.SocketException: The attempted o peration is not supported for the type of object referenced at System.Net.IPAddress.set_ScopeId(Int64 value) --- End of inner exception stack trace ---

This is due to the ScopeId property no being supported all AddressFamily.InterNetwork.

Admittedly, IPAddress has design flaws, but I think it would be nice if AutoFixture knew about those well known flawed types and handled them in an exceptional manner.

[moodmosaic]

I think, it wouldn't be easy to support each and every poorly designed built-in BCL object, because at the end AutoFixture can't really control the direction of the BCL...

IMHO, the easiest way to deal with all these, is by customizing the Fixture instance and/or using custom ISpecimenBuilder objects.

This technique, of using custom functions/instances/objects, also exists in QuickCheck, Quviq QuickCheck for Erlang, FsCheck, and probably others – AFAIK, none of these tools support each and every base data structure OOTB...

[ploeh]

I'm not going to hide the fact that I'm torn on the general issue.

On the one hand, I think it would be a benefit to AutoFixture if, in general, it just works. The more it just works, the better.

On the other hand, I'm concerned that AutoFixture is going to grow. The code base is big enough already.

It's important to understand, though, that the value in any piece of software is not in how easy or difficult it is to develop and maintain, but how easy it is to use.

This makes me lean towards adding explicit support for MailMessage, provided we can come up with some good defaults for the problematic properties or fields. These defaults should be configurable. The implementations of support for Uri and MailAddress already show the way.

As a principle, I don't think AutoFixture should attempt to cover all BCL classes. If, on the other hand, there's a user of AutoFixture who needs to use a particular BCL class, and wants to put in the effort of writing said support into AutoFixture, I think we should accept the contribution.

In other words: feel free to send a pull request for MailMessage 😄

[moodmosaic]

👍

[Pvlerick]

I believe the MailMessage issue will be solved as soon as AutoFixture will be Encoding aware.

I'm happy to take this, unless @tiesmaster wants to do it 😄

However I'll probably ask for some guidances when it comes to design choices as I'm not so familiar with all the inner mechanics.

[ploeh]

Feel free to ask as much as you'd like 👍 😄

[tiesmaster]

@Pvlerick I'd like to pick this up myself, though, I will be leaving to Iceland in a couple of days, so I might not be able to complete this before that. If you're really in a hurry with this, then go ahead and pick this up, otherwise I'd like to complete this ;)

[Pvlerick]

@tiesmaster No I'm not in a hurry at all, knock yourself out 😄