Integrating Unit Testing functionality into an old COM-based IDE

Question

My UnitTesting add-in works nicely, but it has dependencies on a Reflection and a System add-in, which means 3 add-ins loaded into Excel to write and run a VBA unit test, and I've recently decided that was a pain in the neck and didn't quite make VBA unit testing, uh, very much appealing.

So I proceeded to port my VBA code to C#, and implemented the Extensibitity.IDTExtensibility2 interface in a new COM-visible class library project.

At this point it only generates a "Test" menu, and so far only two commands have been implemented - I'd like what I have to be reviewed so I can make adjustments before I make a mess implementing the rest of the features.

For visual reference, this is the generated menu:

At this point, only the "Run/All tests" and "New test module" commands are implemented.

This is the Extension class that I've registered with ProgId "RetailCoderVBE" in my registry:

using System;
using Extensibility;
using Microsoft.Vbe.Interop;
using RetailCoderVBE.VBIDE;
using RetailCoderVBE.UnitTesting;
using System.Runtime.InteropServices;
using RetailCoderVBE.UnitTesting.UI;

namespace RetailCoderVBE
{
    [ComVisible(true)]
    [Guid("8D052AD8-BBD2-4C59-8DEC-F697CA1F8A66")]
    [ProgId("RetailCoderVBE")]
    public class Extension : IDTExtensibility2
    {
        private VBE _vbe;
        private TestMenu _testMenu;

        public void OnAddInsUpdate(ref Array custom)
        {
        }

        public void OnBeginShutdown(ref Array custom)
        {
        }

        public void OnConnection(object Application, ext_ConnectMode ConnectMode, object AddInInst, ref Array custom)
        {
            _vbe = (VBE)Application;
        }

        public void OnStartupComplete(ref Array custom)
        {
            CreateTestMenu();
        }

        private void CreateTestMenu()
        {
            if (_vbe == null)
            {
                throw new InvalidOperationException("Unable to initialize add-in.");
            }

            _testMenu = new TestMenu();
            _testMenu.OnNewTestClass += OnNewUnitTestModule;
            _testMenu.OnRunAllTests += OnRunAllTests;

            _testMenu.Initialize(_vbe);
        }

        void OnRunAllTests(object sender, EventArgs e)
        {
            var engine = new TestEngine();
            using (var view = new TestResultsWindow())
            {
                engine.RunAllTests(_vbe, view);
                view.ShowDialog();
            }
        }

        void OnNewUnitTestModule(object sender, EventArgs e)
        {
            NewUnitTestModuleCommand.NewUnitTestModule(_vbe);
        }

        public void OnDisconnection(ext_DisconnectMode RemoveMode, ref Array custom)
        {
        }
    }
}

The TestMenu class encapsulates all the dirty logic that actually generates the menu:

using System;
using System.Runtime.InteropServices;
using Microsoft.Office.Core;
using Microsoft.Vbe.Interop;

namespace RetailCoderVBE.VBIDE
{
    [ComVisible(false)]
    public class TestMenu
    {
        private CommandBarButton _newTestModuleButton;
        public CommandBarButton NewTestModuleButton { get { return _newTestModuleButton; } }

        private CommandBarButton _runSelectedTestsButton;
        public CommandBarButton RunSelectedTestsButton { get { return _runSelectedTestsButton; } }

        private CommandBarButton _runAllTestsButton;
        public CommandBarButton RunAllTestsButton { get { return _runAllTestsButton; } }

        private CommandBarButton _runFailedTestsButton;
        public CommandBarButton RunFailedTestsButton { get { return _runFailedTestsButton; } }

        private CommandBarButton _runNotRunTestsButton;
        public CommandBarButton RunNotRunTestsButton { get { return _runNotRunTestsButton; } }

        private CommandBarButton _runPassedTestsButton;
        public CommandBarButton RunPassedTestsButton { get { return _runPassedTestsButton; } }

        private CommandBarButton _runRepeatLastRunButton;
        public CommandBarButton RunRepeatLastRunButton { get { return _runRepeatLastRunButton; } }

        private CommandBarButton _windowsTestExplorerButton;
        public CommandBarButton WindowsTestExplorerButton { get { return _windowsTestExplorerButton; } }

        public void Initialize(VBE vbe)
        {
            AddTestMenu(vbe);
        }

        private void AddTestMenu(VBE vbe)
        {
            var menuBarControls = vbe.CommandBars[1].Controls;
            var beforeIndex = FindMenuInsertionIndex(menuBarControls);
            var menu = menuBarControls.Add(Type: MsoControlType.msoControlPopup, Before: beforeIndex, Temporary: true) as CommandBarPopup;
            menu.Caption = "Te&st";

            _newTestModuleButton = AddMenuButton(menu);
            _newTestModuleButton.Caption = "&New test module";
            _newTestModuleButton.FaceId = 472; // code module icon
            _newTestModuleButton.Click += OnNewTestModuleButtonClick;

            AddTestRunMenu(menu);
            AddTestWindowsMenu(menu);
        }

        private void AddTestRunMenu(CommandBarPopup menu)
        {
            var runMenu = menu.Controls.Add(Type: MsoControlType.msoControlPopup) as CommandBarPopup;
            runMenu.Caption = "&Run";
            runMenu.BeginGroup = true;

            _runSelectedTestsButton = AddMenuButton(runMenu);
            _runSelectedTestsButton.Caption = "&Selected tests";
            _runSelectedTestsButton.Click += OnRunSelectedTestsButtonClick;

            _runAllTestsButton = AddMenuButton(runMenu);
            _runAllTestsButton.Caption = "&All tests";
            _runAllTestsButton.FaceId = 186; // a "play" icon
            _runAllTestsButton.Click += OnRunAllTestsButtonClick;

            _runFailedTestsButton = AddMenuButton(runMenu);
            _runFailedTestsButton.Caption = "&Failed tests";
            _runFailedTestsButton.Click += OnRunFailedTestsButtonClick;

            _runNotRunTestsButton = AddMenuButton(runMenu);
            _runNotRunTestsButton.Caption = "&Not run tests";
            _runNotRunTestsButton.Click += OnRunNotRunTestsButtonClick;

            _runPassedTestsButton = AddMenuButton(runMenu);
            _runPassedTestsButton.Caption = "&Passed tests";
            _runPassedTestsButton.Click += OnRunPassedTestsButtonClick;

            _runRepeatLastRunButton = AddMenuButton(runMenu);
            _runRepeatLastRunButton.Caption = "Repeat &last run";
            _runRepeatLastRunButton.Click += OnRepeatLastRunButtonClick;
        }

        private void AddTestWindowsMenu(CommandBarPopup menu)
        {
            _windowsTestExplorerButton = AddMenuButton(menu);
            _windowsTestExplorerButton.Caption = "&Test Explorer";
            _windowsTestExplorerButton.BeginGroup = true;
            _windowsTestExplorerButton.FaceId = 305; // a "document" icon, with a green checkmark and a red cross
            _windowsTestExplorerButton.Click += OnTestExplorerButtonClick;
        }

        private int FindMenuInsertionIndex(CommandBarControls controls)
        {
            for (int i = 1; i <= controls.Count; i++)
            {
                // insert menu before "Window" built-in menu:
                if (controls[i].BuiltIn && controls[i].Caption == "&Window")
                {
                    return i;
                }
            }

            return controls.Count;
        }

        private CommandBarButton AddMenuButton(CommandBarPopup menu)
        {
            return menu.Controls.Add(Type: MsoControlType.msoControlButton) as CommandBarButton;
        }

        public event EventHandler OnNewTestClass;
        void OnNewTestModuleButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnNewTestClass;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRunSelectedTests;
        void OnRunSelectedTestsButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRunSelectedTests;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRunAllTests;
        void OnRunAllTestsButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRunAllTests;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRunFailedTests;
        void OnRunFailedTestsButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRunFailedTests;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRunNotRunTests;
        void OnRunNotRunTestsButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRunNotRunTests;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRunPassedTests;
        void OnRunPassedTestsButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRunPassedTests;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnRepeatLastRun;
        void OnRepeatLastRunButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnRepeatLastRun;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler OnTestExporer;
        void OnTestExplorerButtonClick(CommandBarButton Ctrl, ref bool CancelDefault)
        {
            var handler = OnTestExporer;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }
    }
}

Now if you've noticed, the OnRunAllTests method in the Extension class is implemented directly in the Extension class. It's short, trivial. But it feels like breaking SRP.

On the other hand, I have this NewUnitTestModuleCommand class, that makes the OnNewUnitTestModule handler a one-liner:

using System.Linq;
using System.Runtime.InteropServices;
using Microsoft.Vbe.Interop;
using RetailCoderVBE.Reflection;

namespace RetailCoderVBE.UnitTesting
{
    [ComVisible(false)]
    internal static class NewUnitTestModuleCommand
    {
        private static readonly string TestModuleTemplate = string.Concat(
            "Option Explicit\n",
            "Private Assert As New RetailCoderVBE.AssertClass\n\n",
            "'test explorer searches in standard code modules, for all\n",
            "'public parameterless procedures with a name that starts with \"Test\".\n",
            "Public Sub TestMethod1()\n",
            "    Assert.Inconclusive\n",
            "End Sub\n\n",
            "'...or public parameterless procedures with a @TestMethod marker:\n",
            "'@TestMethod\n",
            "Public Sub TestMethod2()\n",
            "    Assert.Inconclusive\n",
            "End Sub\n\n",
            "'test methods that raise an error will result in an inconclusive test:\n",
            "'@TestMethod\n",
            "Public Sub TestMethod3()\n",
            "    Err.Raise 5\n",
            "End Sub\n\n"
            );

        private static readonly string TestModuleBaseName = "TestModule";

        public static void NewUnitTestModule(VBE vbe)
        {
            var project = vbe.ActiveVBProject;
            Project.EnsureProjectHasReferenceToAddInTypeLib(project);

            var module = project.VBComponents.Add(vbext_ComponentType.vbext_ct_StdModule);
            module.Name = GetNextTestModuleName(project);
            module.CodeModule.AddFromString(TestModuleTemplate);
            module.Activate();
        }

        private static string GetNextTestModuleName(VBProject project)
        {
            var names = Project.GetComponentNames(project);
            var index = names.Count(n => n.StartsWith(TestModuleBaseName)) + 1;

            return string.Concat(TestModuleBaseName, index);
        }
    }
}

I don't like that it's static. In fact I feel like I've overused static classes in this project, and I'd like it to be more... object-oriented.

This is the code-behind for the form I'm using to display the test results:

using System.Runtime.InteropServices;
using System.Windows.Forms;

namespace RetailCoderVBE.UnitTesting.UI
{
    [ComVisible(false)]
    public partial class TestResultsWindow : Form, ITestOutput
    {
        public TestResultsWindow()
        {
            InitializeComponent();
        }

        public void WriteResult(string projectName, string testName, TestResult result)
        {
            testOutputGridView.Rows.Add(projectName, testName, result.Outcome.ToString(), result.Output, result.Duration.ToString() + " ms");
        }
    }
}

Here's the TestEngine class where I'm going to be implementing all the "Run" commands into:

using System.Collections.Generic;
using System.Linq;
using System.Runtime.InteropServices;
using Microsoft.Vbe.Interop;
using RetailCoderVBE.Reflection;

namespace RetailCoderVBE.UnitTesting
{
    [ComVisible(true)]
    public interface ITestOutput
    {
        void WriteResult(string projectName, string testName, TestResult result);
    }

    [ComVisible(false)]
    internal class TestEngine
    {
        public void RunAllTests(VBE vbe, ITestOutput output)
        {
            var projects = vbe.VBProjects.Cast<VBProject>();
            var tests = new List<TestMethod>();
            foreach (var project in projects)
            {
                tests.AddRange(FindTestMethods(project));
            }

            foreach (var test in tests)
            {
                var result = test.Run();
                output.WriteResult(test.ProjectName, test.MethodName, result);
            }
        }

        private IEnumerable<TestMethod> FindTestMethods(VBProject project)
        {
            var modules = project.VBComponents
                                 .Cast<VBComponent>()
                                 .Where(component => component.Type == vbext_ComponentType.vbext_ct_StdModule);
            foreach (var module in modules)
            {
                var publicProcedures = ProjectComponent.GetMembers(module.CodeModule)
                                                       .Where(member => member.MemberType == MemberType.Sub 
                                                                     && member.MemberVisibility == MemberVisibility.Public
                                                                     && member.Signature.EndsWith(member.Name + "()"));

                foreach (var procedure in publicProcedures)
                {
                    if (!procedure.HasAttribute<TestMethodAttribute>() && !procedure.Name.StartsWith("Test"))
                    {
                        continue;
                    }

                    yield return new TestMethod(project.Name, procedure.Name);
                }
            }
        }
    }
}

Answer 1

Looks pretty good. A few minors:

1. You repeat a lot of code for the event raising. An extension method could help to facilitate DRY:

   static public void Raise(this EventHandler handler, object sender)
   {
       if (handler != null)
       {
           handler(sender, EventArgs.Empty);
       }
   }
   

   then you can raise an event like this:

   OnNewTestClass.Raise(this);
   

2. In FindTestMethods you find a set of methods with LINQ yet in the loop you still have this check:

   if (!procedure.HasAttribute<TestMethodAttribute>() && !procedure.Name.StartsWith("Test"))
   {
       continue;
   }
   

   which you could pull up into the Where clause as well. I'd also extract the Where condition into a private method like IsTestMethod and utilize SelectMany to get all members of all components. With all of that the method can be refactored to:

   private IEnumerable<TestMethod> FindTestMethods(VBProject project)
   {
       return project.VBComponents
                     .Cast<VBComponent>()
                     .Where(component => component.Type == vbext_ComponentType.vbext_ct_StdModule)
                     .SelectMany(module => ProjectComponent.GetMembers(module.CodeModule))
                     .Where(IsTestMethod)
                     .Select(procedure => new TestMethod(project.Name, procedure.Name));
   }