Testing abstract SettingConverter with mocks

Question

One of my frameworks uses the ISettingConverter interface as an abstraction for serialization.

public interface ISettingConverter
{
 [NotNull]
 object Deserialize([NotNull] object value, [NotNull] Type targetType);
 [NotNull]
 object Serialize([NotNull] object value);
}

It is implemented by the abstract SettingConverter class that provides logic that prevents unnecessary calls to actual implementations and that can fallback to a default target type for serialization (the first type on the list) if the target does not support the type of the value natively. A target can be a database or an app.config or the windows registry (which natively supports more types then just strings).

public abstract class SettingConverter : ISettingConverter
{
 private readonly ISet<Type> _supportedTypes;
 private readonly Type _fallbackType;
 protected SettingConverter(IEnumerable<Type> supportedTypes)
 {
 _supportedTypes = new HashSet<Type>(supportedTypes ?? throw new ArgumentNullException(nameof(supportedTypes)));
 _fallbackType = supportedTypes.FirstOrDefault() ?? throw new ArgumentException("There must be at least one supprted type.");
 }
 public object Deserialize(object value, Type targetType)
 {
 return
 value.GetType() == targetType 
 ? value 
 : DeserializeCore(value, targetType);
 }
 [NotNull]
 protected abstract object DeserializeCore([NotNull]object value, [NotNull] Type targetType);
 public object Serialize(object value)
 {
 var targetType =
 _supportedTypes.Contains(value.GetType())
 ? value.GetType()
 : _fallbackType;
 return
 value.GetType() == targetType
 ? value
 : SerializeCore(value, targetType);
 }
 [NotNull]
 protected abstract object SerializeCore([NotNull]object value, [NotNull] Type targetType);
}

---

To test this base class I wrote a couple of tests using the free version of the JustMock framework and the MSTest engine with Visual Studio 2017.

---

The first test makes sure that DeserializeCore is not called if the type of the value already has the target type.

 [TestMethod]
 public void Deserialize_ValueHasTargetType_DeserializeCoreNotCalled()
 {
 var settingConverter = Mock.Create<SettingConverter>(Behavior.CallOriginal, (IEnumerable<Type>)new[] { typeof(string) });
 Mock
 .NonPublic
 .Arrange<object>(
 settingConverter, 
 "DeserializeCore", 
 ArgExpr.IsAny<object>(), 
 ArgExpr.IsAny<Type>())
 .OccursNever();
 var result = settingConverter.Deserialize("foo", typeof(string));
 settingConverter.Assert();
 Assert.AreEqual("foo", result);
 }

---

Then I have a test to make sure the opposite, this is, DeserializeCore must be called if the type of the value is not one of the target types. In this case int != string.

 [TestMethod]
 public void Deserialize_ValueHasOtherType_DeserializeCoreCalled()
 {
 var settingConverter = Mock.Create<SettingConverter>(Behavior.CallOriginal, (IEnumerable<Type>)new[] { typeof(string) });
 Mock
 .NonPublic
 .Arrange<object>(
 settingConverter, 
 "DeserializeCore", 
 ArgExpr.IsAny<object>(), 
 ArgExpr.IsAny<Type>())
 .OccursOnce();
 settingConverter.Deserialize("foo", typeof(int));
 settingConverter.Assert();
 }

---

The other two tests make sure that serialization does what it is supposed to. So the first test verifies that SerializeCore is not called because the type of the value alread is the same as one of the types that the target supports.

 [TestMethod]
 public void Serialize_ValueHasSupportedType_SerializeCoreNotCalled()
 {
 var settingConverter = Mock.Create<SettingConverter>(Behavior.CallOriginal, (IEnumerable<Type>)new[] { typeof(string) });
 Mock
 .NonPublic
 .Arrange<object>(
 settingConverter,
 "SerializeCore",
 ArgExpr.IsAny<object>(),
 ArgExpr.IsAny<Type>())
 .OccursNever();
 settingConverter.Serialize("foo");
 settingConverter.Assert();
 }

---

The last test verifies that SerializeCore is called with a fallback type which in this case is the string because int is not supported and must be converted first.

 [TestMethod]
 public void Serialize_ValueHasUnsupportedType_SerializeCoreCalledWithFallbackType()
 {
 var settingConverter = Mock.Create<SettingConverter>(Behavior.CallOriginal, (IEnumerable<Type>)new[] { typeof(string) });
 Mock
 .NonPublic
 .Arrange<object>(
 settingConverter,
 "SerializeCore",
 ArgExpr.IsAny<object>(),
 ArgExpr.Matches<Type>(t => t == typeof(string)))
 .OccursOnce();
 settingConverter.Serialize(123);
 settingConverter.Assert();
 }

---

I'd like you take a look at these tests and tell me how good or bad they are and what would you improve if anything? Is their purpose and implementation clear? Did I pick the right names for everything?

Answer 1

Design

I would argue that this base class doesn't add enough state / operations to be a base class. Consider using a mechanism other than inheritance to accomplish boiler-plate fallback serialisation.

---

Encapsulation

There is some debat whether private methods (is protected considered private in this context?) should even be tested. I like Keith Nicolas' answer why you shouldn't.

The idea of a unit test is to test the unit by its public 'API'.

---

Naming Conventions

• You are following Roy Osherove's naming standards that have format:[UnitOfWork_StateUnderTest_ExpectedBehavior]

• I'm more a fan of the [ShouldDoThis] and [ShouldDoThis_WhenThatOccurs] format. There is a nice Fluent Should API available that could aid testing.

As an example:

Serialize_ValueHasUnsupportedType_SerializeCoreCalledWithFallbackType

vs

ShouldCallSerializeCoreWithFallbackType_WhenCallingSerializeWithUnsupportedType

---

Unit Test should test one thing

Some of your tests (like Deserialize_ValueHasTargetType_DeserializeCoreNotCalled) perform both white-box as black-box tests. We could argue whether these should be split into two separate unit tests; (1) one checking core is not called, and (2) one checking the result it the value specified. If you do decide to keep one unit tests for both assertions, I would change its name to include that the value itself should be returned.

// white-box test
settingConverter.Assert();
// black-box test
Assert.AreEqual("foo", result);

• t3chb0t: Interesting pionts. I didn't know about white-box testing. I now use the feature-oriented naming convention for test methods like Can_find_setting_by_provider_type which I find is the most informative one. I consider the fluent should-api as fluent-overflow ;-]
• dfhwze: of course, features I should have guessed ;-)
• dfhwze: anyway, zombie down!
• t3chb0t: ... let me see, I think I can add a couple of new zombies :-P
• dfhwze: let me have a go at it ..