3
\$\begingroup\$

Recently, I redesigned the Rubberduck settings pane. Here is the unit test settings view, which is posted in this question:

Rubberduck Unit Test Settings View

The part we are referring to is the section in the right pane. The rest of the window is part of a parent control that will be posted in its own question later.

This is the XAML for this window:

<UserControl x:Class="Rubberduck.UI.Settings.UnitTestSettings"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:settings="clr-namespace:Rubberduck.UI.Settings"
             xmlns:settingsConfig="clr-namespace:Rubberduck.Settings"
             xmlns:converters="clr-namespace:Rubberduck.UI.Settings.Converters"
             xmlns:core="clr-namespace:System;assembly=mscorlib"
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="300"
             d:DataContext="{d:DesignInstance {x:Type settings:UnitTestSettingsViewModel}, IsDesignTimeCreatable=False}">
    <UserControl.Resources>
        <converters:BindingModeToTextConverter x:Key="BindingModeToText" />
        <converters:BindingModeValueToTextConverter x:Key="BindingModeValueToText" />
        <converters:AssertModeToTextConverter x:Key="AssertModeToText" />
        <converters:AssertModeValueToTextConverter x:Key="AssertModeValueToText" />

        <ObjectDataProvider x:Key="BindingMode" MethodName="GetValues" ObjectType="{x:Type core:Enum}">
            <ObjectDataProvider.MethodParameters>
                <x:Type Type="settingsConfig:BindingMode"/>
            </ObjectDataProvider.MethodParameters>
        </ObjectDataProvider>
        <ObjectDataProvider x:Key="AssertMode" MethodName="GetValues" ObjectType="{x:Type core:Enum}">
            <ObjectDataProvider.MethodParameters>
                <x:Type Type="settingsConfig:AssertMode"/>
            </ObjectDataProvider.MethodParameters>
        </ObjectDataProvider>
    </UserControl.Resources>
    <Grid>
        <StackPanel Margin="5,5,5,0">
            <Label DockPanel.Dock="Top"
                   Background="DarkGray"
                   Foreground="White"
                   FontWeight="SemiBold"
                   Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=SettingsCaption_UnitTestSettings}"
                   Margin="0,0,0,3">
                <Label.Style>
                    <Style>
                        <Style.Resources>
                            <Style TargetType="{x:Type Border}">
                                <Setter Property="CornerRadius" Value="5"/>
                            </Style>
                        </Style.Resources>
                    </Style>
                </Label.Style>
            </Label>

            <Label Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_BindingMode}"
                   FontWeight="SemiBold" />
            <ComboBox Margin="5,0,0,5"
                      Width="210"
                      HorizontalAlignment="Left"
                      ItemsSource="{Binding Source={StaticResource BindingMode}, Converter={StaticResource BindingModeToText}, UpdateSourceTrigger=PropertyChanged}"
                      SelectedItem="{Binding BindingMode, Converter={StaticResource BindingModeValueToText}}" />

            <Label Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_AssertMode}"
                   FontWeight="SemiBold" />
            <ComboBox Margin="5,0,0,5"
                      Width="210"
                      HorizontalAlignment="Left"
                      ItemsSource="{Binding Source={StaticResource AssertMode}, Converter={StaticResource AssertModeToText}, UpdateSourceTrigger=PropertyChanged}"
                      SelectedItem="{Binding AssertMode, Converter={StaticResource AssertModeValueToText}}" />

            <Label Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_TestModuleTemplateHeader}"
                   FontWeight="SemiBold" />
            <CheckBox Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_IncludeTestModuleInitCleanupPrompt}"
                      IsChecked="{Binding ModuleInit, Mode=TwoWay}"
                      Margin="5,0,0,5"
                      HorizontalAlignment="Left" />
            <CheckBox Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_IncludeTestMethodInitCleanupPrompt}"
                      IsChecked="{Binding MethodInit, Mode=TwoWay}"
                      Margin="5,0,0,5"
                      HorizontalAlignment="Left" />
            <CheckBox Content="{Resx ResxName=Rubberduck.UI.RubberduckUI, Key=UnitTestSettings_IncludeTestMethodStubWhenCreatingModulePrompt}"
                      IsChecked="{Binding DefaultTestStubInNewModule, Mode=TwoWay}"
                      Margin="5,0,0,0"
                      HorizontalAlignment="Left" />
        </StackPanel>
    </Grid>
</UserControl>

This control uses four converters. Two of them convert the IEnumerable<T> given to the comboboxes' ItemsSource to a list of localized strings, and only convert one-way. The other two take the selected item, which is now a string, and convert it back to the specific enum type used by the combobox. This is used to bind the selected item to a value in the view model that more accurately represents the selection; these latter ones must work in both directions. These are uncomfortably similar, but I am not sure how to reduce the similarities:

public class BindingModeToTextConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var modes = (IEnumerable<Rubberduck.Settings.BindingMode>)value;
        return modes.Select(s => RubberduckUI.ResourceManager.GetString("UnitTestSettings_" + s)).ToArray();
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
        return value;
    }
}

public class BindingModeValueToTextConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var mode = (Rubberduck.Settings.BindingMode)value;
        switch (mode)
        {
            case Rubberduck.Settings.BindingMode.EarlyBinding:
                return RubberduckUI.UnitTestSettings_EarlyBinding;
            case Rubberduck.Settings.BindingMode.LateBinding:
                return RubberduckUI.UnitTestSettings_LateBinding;
            default:
                return value;
        }
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var text = (string)value;
        return text == RubberduckUI.UnitTestSettings_EarlyBinding
            ? Rubberduck.Settings.BindingMode.EarlyBinding
            : Rubberduck.Settings.BindingMode.LateBinding;
    }
}

public class AssertModeToTextConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var modes = (IEnumerable<Rubberduck.Settings.AssertMode>)value;
        return modes.Select(s => RubberduckUI.ResourceManager.GetString("UnitTestSettings_" + s)).ToArray();
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
        return value;
    }
}

public class AssertModeValueToTextConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var mode = (Rubberduck.Settings.AssertMode)value;
        switch (mode)
        {
            case Rubberduck.Settings.AssertMode.StrictAssert:
                return RubberduckUI.UnitTestSettings_StrictAssert;
            case Rubberduck.Settings.AssertMode.PermissiveAssert:
                return RubberduckUI.UnitTestSettings_PermissiveAssert;
            default:
                return value;
        }
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
        var text = (string)value;
        return text == RubberduckUI.UnitTestSettings_StrictAssert
            ? Rubberduck.Settings.AssertMode.StrictAssert
            : Rubberduck.Settings.AssertMode.PermissiveAssert;
    }
}

The code-behind only exposes the view model:

public partial class UnitTestSettings : ISettingsView
{
    public UnitTestSettings()
    {
        InitializeComponent();
    }

    public UnitTestSettings(ISettingsViewModel vm)
        : this()
    {
        DataContext = vm;
    }

    public ISettingsViewModel ViewModel { get { return DataContext as ISettingsViewModel; } }
}

Here is the view model:

public class UnitTestSettingsViewModel : ViewModelBase, ISettingsViewModel
{
    public UnitTestSettingsViewModel(Configuration config)
    {
        BindingMode = config.UserSettings.UnitTestSettings.BindingMode;
        AssertMode = config.UserSettings.UnitTestSettings.AssertMode;
        ModuleInit = config.UserSettings.UnitTestSettings.ModuleInit;
        MethodInit = config.UserSettings.UnitTestSettings.MethodInit;
        DefaultTestStubInNewModule = config.UserSettings.UnitTestSettings.DefaultTestStubInNewModule;
    }

    #region Properties

    private BindingMode _bindingMode;
    public BindingMode BindingMode
    {
        get { return _bindingMode; }
        set
        {
            if (_bindingMode != value)
            {
                _bindingMode = value;
                OnPropertyChanged();
            }
        }
    }

    private AssertMode _assertMode;
    public AssertMode AssertMode
    {
        get { return _assertMode; }
        set
        {
            if (_assertMode != value)
            {
                _assertMode = value;
                OnPropertyChanged();
            }
        }
    }

    private bool _moduleInit;
    public bool ModuleInit
    {
        get { return _moduleInit; }
        set
        {
            if (_moduleInit != value)
            {
                _moduleInit = value;
                OnPropertyChanged();
            }
        }
    }

    private bool _methodInit;
    public bool MethodInit
    {
        get { return _methodInit; }
        set
        {
            if (_methodInit != value)
            {
                _methodInit = value;
                OnPropertyChanged();
            }
        }
    }

    private bool _defaultTestStubInNewModule;
    public bool DefaultTestStubInNewModule
    {
        get { return _defaultTestStubInNewModule; }
        set
        {
            if (_defaultTestStubInNewModule != value)
            {
                _defaultTestStubInNewModule = value;
                OnPropertyChanged();
            }
        }
    }

    #endregion

    public void UpdateConfig(Configuration config)
    {
        config.UserSettings.UnitTestSettings.BindingMode = BindingMode;
        config.UserSettings.UnitTestSettings.AssertMode = AssertMode;
        config.UserSettings.UnitTestSettings.ModuleInit = ModuleInit;
        config.UserSettings.UnitTestSettings.MethodInit = MethodInit;
        config.UserSettings.UnitTestSettings.DefaultTestStubInNewModule = DefaultTestStubInNewModule;
    }

    public void SetToDefaults(Configuration config)
    {
        BindingMode = config.UserSettings.UnitTestSettings.BindingMode;
        AssertMode = config.UserSettings.UnitTestSettings.AssertMode;
        ModuleInit = config.UserSettings.UnitTestSettings.ModuleInit;
        MethodInit = config.UserSettings.UnitTestSettings.MethodInit;
        DefaultTestStubInNewModule = config.UserSettings.UnitTestSettings.DefaultTestStubInNewModule;
    }
}

Finally, here are my unit tests for the view model, which have 100% coverage. Two of these tests test methods that make broad changes, so I use multiple Asserts to ensure everything is working. Is this OK, or should I split these methods up and test only one output of these methods at a time, or do something else?

private Configuration GetDefaultConfig()
{
    var unitTestSettings = new UnitTestSettings()
    {
        BindingMode = BindingMode.LateBinding,
        AssertMode = AssertMode.StrictAssert,
        ModuleInit = true,
        MethodInit = true,
        DefaultTestStubInNewModule = false
    };

    var userSettings = new UserSettings(null, null, null, unitTestSettings, null);
    return new Configuration(userSettings);
}

private Configuration GetNondefaultConfig()
{
    var unitTestSettings = new UnitTestSettings()
    {
        BindingMode = BindingMode.EarlyBinding,
        AssertMode = AssertMode.PermissiveAssert,
        ModuleInit = false,
        MethodInit = false,
        DefaultTestStubInNewModule = true
    };

    var userSettings = new UserSettings(null, null, null, unitTestSettings, null);
    return new Configuration(userSettings);
}

[TestMethod]
public void SaveConfigWorks()
{
    var viewModel = new UnitTestSettingsViewModel(GetNondefaultConfig());
    viewModel.UpdateConfig(GetNondefaultConfig());

    Assert.AreEqual(BindingMode.EarlyBinding, viewModel.BindingMode);
    Assert.AreEqual(AssertMode.PermissiveAssert, viewModel.AssertMode);
    Assert.AreEqual(false, viewModel.ModuleInit);
    Assert.AreEqual(false, viewModel.MethodInit);
    Assert.AreEqual(true, viewModel.DefaultTestStubInNewModule);
}

[TestMethod]
public void SetDefaultsWorks()
{
    var viewModel = new UnitTestSettingsViewModel(GetNondefaultConfig());

    viewModel.SetToDefaults(GetDefaultConfig());

    Assert.AreEqual(BindingMode.LateBinding, viewModel.BindingMode);
    Assert.AreEqual(AssertMode.StrictAssert, viewModel.AssertMode);
    Assert.AreEqual(true, viewModel.ModuleInit);
    Assert.AreEqual(true, viewModel.MethodInit);
    Assert.AreEqual(false, viewModel.DefaultTestStubInNewModule);
}

[TestMethod]
public void BindingModeIsSetInCtor()
{
    var viewModel = new UnitTestSettingsViewModel(GetDefaultConfig());

    Assert.AreEqual(BindingMode.LateBinding, viewModel.BindingMode);
}

[TestMethod]
public void AssertModeIsSetInCtor()
{
    var viewModel = new UnitTestSettingsViewModel(GetDefaultConfig());

    Assert.AreEqual(AssertMode.StrictAssert, viewModel.AssertMode);
}

[TestMethod]
public void ModuleInitIsSetInCtor()
{
    var viewModel = new UnitTestSettingsViewModel(GetDefaultConfig());

    Assert.AreEqual(true, viewModel.ModuleInit);
}

[TestMethod]
public void MethodInitIsSetInCtor()
{
    var viewModel = new UnitTestSettingsViewModel(GetDefaultConfig());

    Assert.AreEqual(true, viewModel.MethodInit);
}

[TestMethod]
public void DefaultTestStubInNewModuleIsSetInCtor()
{
    var viewModel = new UnitTestSettingsViewModel(GetDefaultConfig());

    Assert.AreEqual(false, viewModel.DefaultTestStubInNewModule);
}

For reference, this is how the SaveConfig() method is called:

foreach (var vm in SettingsViews.Select(v => v.Control.ViewModel))
{
    vm.UpdateConfig(_config);
}

Passing the configuration needed to be updated into the method ensures that the other settings are saved as well because the only settings updated are the ones related the unit test settings. Because the configuration passed in has reference semantics, the parameter passed in keeps the changes.

\$\endgroup\$
4
\$\begingroup\$

In this answer I would focus on only one area, and that is one vs multiple asserts in any given test method. The normal design suggestion is to follow the pattern of Act, Arrange and Assert, and that each method should only have one assertion.

In most cases this sound advice, and should be followed. However, I've found that in some cases it can be very useful to do multiple tests when you build larger objects (or view models). In other words doing multiple asserts can be good when there is a relative high cost in building or arranging your test.

The downside however of having multiple asserts in one test method, is that it fails one the first method, and thusly may hide other failures in the latter asserts. This calls for a rather expensive test, fix first assert failure, re-test, fix second assert failure, and so on.

Luckily, there exist a rather good alternative which I've been using for some years in different contexts, and that is to have Multiple asserts in a single unit test method. I modified this a little in our projects, so as to handle messages slightly better, but this leads to changing this code:

[TestMethod]
public void SetDefaultsWorks()
{
    var viewModel = new UnitTestSettingsViewModel(GetNondefaultConfig());

    viewModel.SetToDefaults(GetDefaultConfig());

    Assert.AreEqual(BindingMode.LateBinding, viewModel.BindingMode);
    Assert.AreEqual(AssertMode.StrictAssert, viewModel.AssertMode);
    Assert.AreEqual(true, viewModel.ModuleInit);
    Assert.AreEqual(true, viewModel.MethodInit);
    Assert.AreEqual(false, viewModel.DefaultTestStubInNewModule);
}

Into something like the following, where I've also changed to using IsTrue(...) instead of AreEqual(true, ...) as I feel it is more directly conveys expected result:

[TestMethod]
public void SetDefaultsWorks()
{
    var viewModel = new UnitTestSettingsViewModel(GetNondefaultConfig());

    viewModel.SetToDefaults(GetDefaultConfig());

    MultiAssert.Aggregate(
        () => Assert.AreEqual(BindingMode.LateBinding, viewModel.BindingMode),
        () => Assert.AreEqual(AssertMode.StrictAssert, viewModel.AssertMode),
        () => Assert.IsTrue(viewModel.ModuleInit),
        () => Assert.IsTrue(viewModel.MethodInit),
        () => Assert.IsFalse(viewModel.DefaultTestStubInNewModule)
        );
}

If everything is good, the test displays as OK in whatever test environment you use. But if one or more of them fail, then all the failures will display, and you can correct all fails in one go, and you'll also get a fuller view of why this test failed.

Addendum on SaveConfigWorks

As discussed in chat, your SaveConfigWorks which also could benefit from using MultiAssert tests whether the newly created Configuration object is changed from the object returned by GetNondefaultConfig() to essential the same object. I would strongly suggest to actually change it to using a temporary object in the test method, and to actually verify the changes of this Configuration object.

\$\endgroup\$
  • \$\begingroup\$ Thanks. I meant to comment on how I was uncomfortable with the multiple asserts in the question, but I forgot. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Feb 17 '16 at 1:43
  • \$\begingroup\$ You made me realize that I was never testing the changes to the config in my test for updating the config. Thanks. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Feb 17 '16 at 2:08
  • \$\begingroup\$ I cannot imagine to have only one assert per test. This would be totally crazy. \$\endgroup\$ – t3chb0t Sep 4 '16 at 4:55
2
\$\begingroup\$

Tests can have setup and teardown methods that are executed pre and post tests.

[TestInitialize]
public void Setup()
{
    _viewModel = new UnitTestSettingsViewModel(GetNondefaultConfig()); //or
    _viewModel = Mock.Of<IUnitTestSettingsViewModel>(...
}

If you are using FluentAssertions then instead of MultiAssert you would get readable error messages and could just say

config.ShouldBeEquivalentTo(viewModel.config);

With moq you could verify that save config calls SetConfig internally

    [TestMethod]
    public void SaveConfig_Should_Set_ViewModel_Config()
    {
        var expectedConfig = GetDefaultConfig();
        Mock.Get(viewModelService)
            .Setup(o => o.SetConfig(It.IsAny<Configuration>()))
            .Returns(config);

        // Act
        _viewModel.UpdateConfig(config);

        // Assert
        Mock.Get(viewModelService)
            .Verify(o => o.SetConfig(expectedConfig), Times.Once);
    }

Personally, I think event driven methods are much easier to read than converters, but I don't like to force my opinion on others so I digress.

\$\endgroup\$
  • \$\begingroup\$ I don't think I can use an event-driven method here because I am passing one value to my XAML, but my XAML needs it in a different format. I considered using Moq, but I decided I don't need a mocked value, I just need to ensure the output for the given input is right. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Feb 17 '16 at 16:37
1
\$\begingroup\$

In addition to holroy's comments, I should not hard-code the values into my method and into the test. I should write my tests to compare the expected value against the value in the configuration I am testing against:

var defaultConfig = GetDefaultConfig();
viewModel = new UnitTestSettingsViewModel(defaultConfig);

Assert.AreEqual(defaultConfig.UserSettings.UnitTestSettings.BindingMode, viewModel.BindingMode);

This ensures that tests will not break if a value in the config generator gets changed.


Additionally, when I test whether the config saves, I only run the method, I am actually testing whether the save config updates the view model properties, rather than testing that the config passed in actually gets updated. This is the correct version (including the MultiBinding as suggested):

[TestMethod]
public void SaveConfigWorks()
{
    var customConfig = GetNondefaultConfig();
    var viewModel = new UnitTestSettingsViewModel(customConfig);

    var config = GetDefaultConfig();
    viewModel.UpdateConfig(config);

    MultiAssert.Aggregate(
        () => Assert.AreEqual(config.UserSettings.UnitTestSettings.BindingMode, viewModel.BindingMode),
        () => Assert.AreEqual(config.UserSettings.UnitTestSettings.AssertMode, viewModel.AssertMode),
        () => Assert.AreEqual(config.UserSettings.UnitTestSettings.ModuleInit, viewModel.ModuleInit),
        () => Assert.AreEqual(config.UserSettings.UnitTestSettings.MethodInit, viewModel.MethodInit),
        () => Assert.AreEqual(config.UserSettings.UnitTestSettings.DefaultTestStubInNewModule, viewModel.DefaultTestStubInNewModule));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.