Unit Test Settings

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

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:

This control uses four converters. Two of them convert the IEnumerable 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)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

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.

StackExchange Code Review Q#120254, answer score: 4