Remember: 🔴 red, 🟢 green, ♻️ refactor
One of the things that Dave Farley says is that you should not mock your external dependencies and you should treat your framework as an edge of your system. In an Umbraco site, that means you should make abstractions for the parts of your system that touch Umbraco. So... I tried it and found that it's easier said than done.
The challenge: Published Content
The assignment is pretty simple: "given a content page, find more relevant content". This feature is heavily dependent on Umbraco in several ways:
- The input is a content page from Umbraco with control options
- The output is a list of more content pages from Umbraco
- Pages need to be found using either content tree traversal or an index search
I reviewed these constraints and decided that it wasn't a good idea to create an abstraction for IPublishedContent. Instead, I decided to go against the learnings of TDD and mock the published content model.
First attempt: mocking published content with a library
The IPublishedContent interface has 22 properties that need to be implemented, though in practice I really only need about 3 properties for testing. I could've chosen to mock the interface with a library. A test with NSubstitute could look like this:
[Fact]
public void ShouldReturnNullWhenRelatedContentIsDisabled()
{
// given
var testProperty = Substitute.For<IPublishedProperty>();
testProperty.GetValue().Returns(false);
var testContent = Substitute.For<IPublishedContent>();
testContent.GetProperty(Arg.Is<string>("enabled")).Returns(testProperty);
// when
var result = _sut.GetRelatedContent(new ContentPage(testContent, Substitute.For<IPublishedValueFallback>()));
// then
Assert.IsNull(result);
}
Can you turn this test into an English sentence? Can you tell what the startcondition is? Perhaps you can, but this code is difficult to read. We can make something much better.
Second attempt: Custom fake published content
Our content is going to be a fancy dictionary to which we can assign property values by alias. Published content is separated in content and elements. Content inherits all properties from elements, so we can save a few lines by mimicking this inheritance structure.
FakePublishedProperty.cs
internal sealed class FakePublishedProperty(string alias, object? value)
: IPublishedProperty
{
public IPublishedPropertyType PropertyType => throw new NotImplementedException();
public string Alias { get; } = alias;
public object? GetDeliveryApiValue(bool expanding, string? culture = null, string? segment = null)
{
return value;
}
public object? GetSourceValue(string? culture = null, string? segment = null)
{
return value;
}
public object? GetValue(string? culture = null, string? segment = null)
{
return value;
}
public object? GetXPathValue(string? culture = null, string? segment = null)
{
return null;
}
public bool HasValue(string? culture = null, string? segment = null)
{
return value is not null;
}
}
FakePublishedElement.cs
internal class FakePublishedElement(Guid? key = null)
: IPublishedElement
{
private readonly Dictionary<string, IPublishedProperty> _properties = [];
public IPublishedContentType ContentType => throw new NotImplementedException();
public Guid Key { get; } = key ?? Guid.NewGuid();
public IEnumerable<IPublishedProperty> Properties => _properties.Values;
public IPublishedProperty? GetProperty(string alias)
{
return _properties.TryGetValue(alias, out var prop)
? prop
: null;
}
public void AssignPropertyValue(string alias, object? value)
{
_properties[alias] = new FakePublishedProperty(alias, value);
}
public void UnassignPropertyValue(string alias)
{
_properties.Remove(alias);
}
}
FakePublishedContent.cs
internal sealed class FakePublishedContent(int id, string name)
: FakePublishedElement, IPublishedContent
{
public int Id { get; } = id;
public string Name { get; } = name;
// ... the rest of the interface is simply implemented with { get; set; } and default values
}
If we rewrite the test with this new model, it will look like this:
[Fact]
public void ShouldReturnNullWhenRelatedContentIsDisabled()
{
// given
var testContent = new FakePublishedContent(1234, "Content page");
testContent.AssignPropertyValue("enabled", false);
// when
var result = _sut.GetRelatedContent(new ContentPage(testContent, Substitute.For<IPublishedValueFallback>()));
// then
Assert.IsNull(result);
}
Much better already. The "given" part is much more readable and you can now easily understand the starting condition.
Strong typing
One of Umbraco's strong features is its modelsbuilder. The modelsbuilder creates an abstraction on top of the property aliasses so that we don't need to rely on string values, which can break easily without noticing. So why should we still rely on string aliasses in our tests?
A small builder inside the fake published element allows us to create our own abstraction (and will help us a little later as well):
PropertyAliasHelper.cs
internal static class PropertyAliasHelper
{
// Calling this function allows us to get the string alias, based on a strongly typed expression, like this:
// PropertyAliasHelper.AliasOf<ContentPage, bool>(c => c.Enabled);
public static string AliasOf<TContent, TProp>(Expression<Func<TContent, TProp?>> expression)
where TContent : IPublishedElement
{
if (expression.Body is not MemberExpression member)
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture,
"Expression '{0}' refers to a method, not a property.",
expression.ToString()));
}
if (member.Member is not PropertyInfo propInfo)
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture,
"Expression '{0}' refers to a field, not a property.",
expression.ToString()));
}
Type type = typeof(TContent);
if (propInfo.ReflectedType != null && type != propInfo.ReflectedType && !type.IsSubclassOf(propInfo.ReflectedType))
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture,
"Expression '{0}' refers to a property that is not from type {1}.",
expression.ToString(),
type));
}
return propInfo.GetCustomAttribute<ImplementPropertyTypeAttribute>()!.Alias;
}
}
FakePublishedElement.cs
internal class FakePublishedElement(Guid? key = null)
: IPublishedElement
{
// ... all previously implemented properties
// 👇 Add a small helper method to wrap the fake content inside a modelsbuilder model
public T WrapIn<T>()
where T : IPublishedElement
{
if (Activator.CreateInstance(typeof(T), this, Substitute.For<IPublishedValueFallback>()) is not T result)
throw new InvalidOperationException($"Unable to create published content of type {typeof(T)}");
return result;
}
// 👇 Call this method to fluently and strongly assign values to properties within a specific content type
public PropertyValueBuilder<TContent> PropertyValuesFor<TContent>()
where TContent : IPublishedElement
=> new(this);
public sealed class PropertyValueBuilder<TContent>(FakePublishedElement content)
where TContent : IPublishedElement
{
public PropertyValueBuilder<TContent> Set<TProp>(Expression<Func<TContent, TProp?>> propertyExpression, TProp? value)
{
content.AssignPropertyValue(PropertyAliasHelper.AliasOf(propertyExpression), value);
return this;
}
public PropertyValueBuilder<TContent> Unset<TProp>(Expression<Func<TContent, TProp?>> propertyExpression)
{
content.UnassignPropertyValue(PropertyAliasHelper.AliasOf(propertyExpression));
return this;
}
}
}
If we rewrite the original test now, it looks like this:
[Fact]
public void ShouldReturnNullWhenRelatedContentIsDisabled()
{
// given
var testContent = new FakePublishedContent(1234, "Content page");
testContent.PropertyValuesFor<ContentPage>()
.Set(p => p.Enabled, true);
// when
var result = _sut.GetRelatedContent(testContent.WrapIn<ContentPage>());
// then
Assert.IsNull(result);
}
Is it better? I guess that's a matter of preference at this point. I find strong typing very important, so I think it's better. This code won't compile if content changes in Umbraco.
Final polish
One thing I always tell my coworkers is that you need to clearly convey your intentions in your code. This is especially important in automated tests, because many of my coworkers aren't familiar with them yet. The test as it is written right now, may lead a developer to believe that they need to wrap their content in a modelsbuilder model themselves before passing it to the method. We can create an extension that allows us to work directly on a modelsbuilder model. That way, the code really reads as though we're working directly with a modelsbuilder model:
FakePublishedContentExtensions.cs
internal static class FakePublishedContentExtensions
{
public static PropertyValueBuilder<TContent> PropertyValues<TContent>(this TContent content)
where TContent : IPublishedElement
{
return content switch
{
FakePublishedElement el => el.PropertyValuesFor<TContent>(),
PublishedContentWrapped el => el.PropertyValuesFor<TContent>(),
PublishedElementWrapped el => el.PropertyValuesFor<TContent>(),
_ => throw new InvalidOperationException("Cannot create property value builder from the given content item")
};
}
public static PropertyValueBuilder<TContent> PropertyValuesFor<TContent>(this PublishedContentWrapped content)
where TContent : IPublishedElement
{
// 👇 We unwrap the published content until we find the fake implementation that it is based on.
// It looks like we're working on the modelsbuilder model, but we're actually working on the fake model.
var contentBase = content.Unwrap();
if (contentBase is FakePublishedElement fakeElement) return fakeElement.PropertyValuesFor<TContent>();
if (contentBase is PublishedContentWrapped wrapper) return wrapper.PropertyValuesFor<TContent>();
throw new InvalidOperationException("Cannot get property value builder, because this object is not based on fake content");
}
public static PropertyValueBuilder<TContent> PropertyValuesFor<TContent>(this PublishedElementWrapped content)
where TContent : IPublishedElement
{
var contentBase = content.Unwrap();
if (contentBase is FakePublishedElement fakeElement) return fakeElement.PropertyValuesFor<TContent>();
if (contentBase is PublishedElementWrapped wrapper) return wrapper.PropertyValuesFor<TContent>();
throw new InvalidOperationException("Cannot get property value builder, because this object is not based on fake content");
}
}
Now finally, we can rewrite the test one last time and make it look like this:
[Fact]
public void ShouldReturnNullWhenRelatedContentIsDisabled()
{
// given
var contentPage = new FakePublishedContent(1234, "Content page").WrapIn<ContentPage>();
contentPage.PropertyValues().Set(p => p.Enabled, true);
// when
var result = _sut.GetRelatedContent(contentPage);
// then
Assert.IsNull(result);
}
Retrospect
I started with a test that was extremely verbose and difficult to read. After a set of refactors, I ended up with a test that is short and expressive.
Was it worth it though? In order to make the test this readable, I had to do a significant amount of plumbing. I created fake elements, fake property values, a property builder and several extensions. The test of 4 lines of code, requires around 200 lines of code behind the scenes to actually work.
If it was just this single test, I would've said no. However, I know I will be writing many more tests with published content. Being able to write short, readable tests should be a massive time-saver long term... I hope.
Top comments (0)