TestExplorerWindow, the event-tossing UI

Question

Following up on Integrating Unit Testing functionality into an old COM-based IDE, I made quite a few changes to the UI, and now the code-behind for my TestExplorerWindow is starting to feel somewhat bloated, at least compared to what it was... I blame the added toolbar and the fancypants progressbar.

At least it looks pretty:

Here's the less pretty code-behind - the WriteResult method is the ITestOutput interface implementation - I'm starting to question whether it's needed at all, and whether I actually need all these events:

namespace RetailCoderVBE.UnitTesting.UI
{
    internal partial class TestExplorerWindow : Form, ITestOutput
    {
        private BindingList<TestExplorerItem> _allTests; // all tests found in solution
        private IList<TestExplorerItem> _tests; // tests to execute

        public TestExplorerWindow()
        {
            _allTests = new BindingList<TestExplorerItem>();
            _tests = new List<TestExplorerItem>();

            InitializeComponent();
            InitializeGrid();
            RegisterUIEvents();
        }

        private void InitializeGrid()
        {
            testOutputGridView.DataSource = _allTests;

            var messageColumn = testOutputGridView.Columns
                                                  .Cast<DataGridViewColumn>()
                                                  .FirstOrDefault(column => column.HeaderText == "Message");
            if (messageColumn != null)
            {
                messageColumn.AutoSizeMode = DataGridViewAutoSizeColumnMode.Fill;
            }
        }

        private void RegisterUIEvents()
        {
            FormClosing += TestExplorerWindowFormClosing;

            testOutputGridView.CellDoubleClick += GridCellDoubleClicked;
            testOutputGridView.SelectionChanged += GridSelectionChanged;

            gotoSelectionButton.Click += GotoSelectionButtonClicked;
            addTestMethodButton.Click += AddTestMethodButtonClicked;
            addTestModuleButton.Click += AddTestModuleButtonClicked;
            addExpectedErrorTestMethodButton.Click += AddExpectedErrorTestMethodButtonClicked;
            runAllTestsMenuItem.Click += RunAllTestsMenuItemClicked;
            runFailedTestsMenuItem.Click += RunFailedTestsMenuItemClicked;
            runPassedTestsMenuItem.Click += RunPassedTestsMenuItemClicked;
            runNotRunTestsMenuItem.Click += RunNotRunTestsMenuItemClicked;
            runLastRunMenuItem.Click += RunLastRunMenuItemClicked;
            runSelectedTestMenuItem.Click += RunSelectedTestMenuItemClicked;
        }

        void GridSelectionChanged(object sender, EventArgs e)
        {
            runSelectedTestMenuItem.Enabled = testOutputGridView.SelectedRows.Count != 0;
        }

        private void OnButtonClick(EventHandler clickEvent)
        {
            var handler = clickEvent;
            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }

        public event EventHandler<SelectedTestEventArgs> OnRunSelectedTestButtonClick;
        void RunSelectedTestMenuItemClicked(object sender, EventArgs e)
        {
            var handler = OnRunSelectedTestButtonClick;
            if (handler != null && _allTests.Any())
            {
                var selection = _allTests.Where(test => testOutputGridView.SelectedRows
                                                                          .Cast<DataGridViewRow>()
                                                                          .Select(row => row.DataBoundItem as TestExplorerItem)
                                                                          .Select(item => item.GetTestMethod())
                                                                          .Contains(test.GetTestMethod()));

                handler(this, new SelectedTestEventArgs(selection));
            }
        }

        public event EventHandler OnRunLastRunTestsButtonClick;
        void RunLastRunMenuItemClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnRunLastRunTestsButtonClick);
        }

        public event EventHandler OnRunNotRunTestsButtonClick;
        void RunNotRunTestsMenuItemClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnRunNotRunTestsButtonClick);
        }

        public event EventHandler OnRunPassedTestsButtonClick;
        void RunPassedTestsMenuItemClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnRunPassedTestsButtonClick);
        }

        public event EventHandler OnRunFailedTestsButtonClick;
        void RunFailedTestsMenuItemClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnRunFailedTestsButtonClick);
        }

        public event EventHandler OnRunAllTestsButtonClick;
        void RunAllTestsMenuItemClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnRunAllTestsButtonClick);
        }

        public event EventHandler OnAddExpectedErrorTestMethodButtonClick;
        void AddExpectedErrorTestMethodButtonClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnAddExpectedErrorTestMethodButtonClick);
        }

        public event EventHandler OnAddTestMethodButtonClick;
        void AddTestMethodButtonClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnAddTestMethodButtonClick);
        }

        public event EventHandler OnAddTestModuleButtonClick;
        void AddTestModuleButtonClicked(object sender, EventArgs e)
        {
            OnButtonClick(OnAddTestModuleButtonClick);
        }

        void TestExplorerWindowFormClosing(object sender, FormClosingEventArgs e)
        {
            e.Cancel = true;
            Hide();
        }

        public void ClearProgress()
        {
            _completedCount = 0;
            testProgressBar.Maximum = _allTests.Count;
            testProgressBar.Value = 0;
            UpdateCompletedTestsLabels();
        }

        public void ClearResults()
        {
            _allTests = new BindingList<TestExplorerItem>(_allTests.Select(test => new TestExplorerItem(test.GetTestMethod(), null)).ToList());
            testOutputGridView.DataSource = _allTests;
        }

        private int _completedCount; 
        private void UpdateProgress()
        {
            UpdateCompletedTestsLabels();

            runPassedTestsMenuItem.Enabled = _tests.Any(test => test.Outcome == TestOutcome.Succeeded.ToString());
            runFailedTestsMenuItem.Enabled = _tests.Any(test => test.Outcome == TestOutcome.Failed.ToString());

            testProgressBar.Maximum = _tests.Count;
            testProgressBar.Value = ++_completedCount;

            runLastRunMenuItem.Enabled = _completedCount > 0;
        }

        private void UpdateCompletedTestsLabels()
        {
            passedTestsLabel.Text = string.Format("{0} Passed", _tests.Count(item => item.Outcome == TestOutcome.Succeeded.ToString()));
            failedTestsLabel.Text = string.Format("{0} Failed", _tests.Count(item => item.Outcome == TestOutcome.Failed.ToString()));
            inconclusiveTestsLabel.Text = string.Format("{0} Inconclusive", _tests.Count(item => item.Outcome == TestOutcome.Inconclusive.ToString()));
        }

        private TestExplorerItem FindItem(IEnumerable<TestExplorerItem> items, TestMethod test)
        {
            return items.FirstOrDefault(item => item.ProjectName == test.ProjectName
                                                 && item.ModuleName == test.ModuleName
                                                 && item.MethodName == test.MethodName);
        }

        public void Refresh(IDictionary<TestMethod,TestResult> tests)
        {
            _allTests = new BindingList<TestExplorerItem>(tests.Select(test => new TestExplorerItem(test.Key, test.Value)).ToList());
            testOutputGridView.DataSource = _allTests;
            testOutputGridView.Refresh();
        }

        public void SetPlayList(IDictionary<TestMethod,TestResult> tests)
        {
            _tests = tests.Select(test => new TestExplorerItem(test.Key, test.Value)).ToList();
            UpdateCompletedTestsLabels();
        }

        public event EventHandler OnRefreshListButtonClick;
        private void RefreshTestsButtonClick(object sender, System.EventArgs e)
        {
            OnButtonClick(OnRefreshListButtonClick);
        }

        public event EventHandler<SelectedTestEventArgs> OnGoToSelectedTest;
        private void GridCellDoubleClicked(object sender, DataGridViewCellEventArgs e)
        {
            var handler = OnGoToSelectedTest;
            if (handler != null)
            {
                handler(this, new SelectedTestEventArgs(_allTests[e.RowIndex]));
            }
        }

        void GotoSelectionButtonClicked(object sender, EventArgs e)
        {
            var handler = OnGoToSelectedTest;
            if (handler != null && _allTests.Any())
            {
                handler(this, new SelectedTestEventArgs(_allTests[testOutputGridView.SelectedRows.Cast<DataGridViewRow>().First().Index]));
            }
        }

        public void WriteResult(TestMethod test, TestResult result)
        {
            var gridItem = FindItem(_allTests, test);
            var playListItem = FindItem(_tests, test);

            if (gridItem == null)
            {
                var item = new TestExplorerItem(test, result);
                _allTests.Add(item);
            }
            else
            {
                gridItem.SetResult(result);
                playListItem.SetResult(result);
            }

            UpdateProgress();
            testOutputGridView.Refresh();
        }    
    }

Most events are EventHandler events, but some are EventHandler<SelectedTestEventArgs> - here's the SelectedTestEventArgs class:

internal class SelectedTestEventArgs : EventArgs
{
    public SelectedTestEventArgs(IEnumerable<TestExplorerItem> items)
    {
        _selection = items.Select(item => item.GetTestMethod());
    }

    public SelectedTestEventArgs(TestExplorerItem item)
        : this(new[] { item })
    { }

    private readonly IEnumerable<TestMethod> _selection;
    public IEnumerable<TestMethod> Selection { get { return _selection; } }
}

The TestExplorerItem class represents an item in the grid view:

internal class TestExplorerItem
{
    public TestExplorerItem(TestMethod test, TestResult result)
    {
        _test = test;
        _result = result;
    }

    private readonly TestMethod _test;
    public TestMethod GetTestMethod()
    {
        return _test;
    }

    private TestResult _result;
    public void SetResult(TestResult result)
    {
        _result = result;
    }

    public Image Result { get { return _result.Icon(); } }
    public string ProjectName { get { return _test.ProjectName; } }
    public string ModuleName { get { return _test.ModuleName; } }
    public string MethodName { get { return _test.MethodName; } }
    public string Outcome { get { return _result == null ? string.Empty : _result.Outcome.ToString(); } }
    public string Message { get { return _result == null ? string.Empty : _result.Output; } }
    public string Duration { get { return _result == null ? string.Empty : _result.Duration.ToString() + " ms"; } }
}

And I've written an extension method to convert a TestResult to an icon, fetched from project resources:

internal static class TestResultExtensions
{
    public static Image Icon(this TestResult result)
    {
        var image = RetailCoderVBE.Properties.Resources.question_white;
        if (result != null)
        {
            switch (result.Outcome)
            {
                case TestOutcome.Succeeded:
                    image = RetailCoderVBE.Properties.Resources.tick_circle;
                    break;

                case TestOutcome.Failed:
                    image = RetailCoderVBE.Properties.Resources.minus_circle;
                    break;

                case TestOutcome.Inconclusive:
                    image = RetailCoderVBE.Properties.Resources.exclamation_circle;
                    break;
            }
        }

        return image;
    }
}

The rest of the project can be found on GitHub.

Answer 1

And I've written an extension method to convert a TestResult to an icon, fetched from project resources

This is an abuse of extension methods. TestResult is a class that you wrote. Extensions are for extending classes that we can't modify. There's no reason to write an extension method for a class that you have complete control over. TestResult should implement this directly.

I'm going to risk some general hand waving here, but...

In my mind, anything that's inheriting from Form or Control should be dead dumb simple. It should raise events and provide some properties that a separate class can set. Those properties are then wired into the actual UI for display. There's should be very little logic at this level of the application. That logic isn't easily testable when it's bound up in your UI.

A more proper architecture would look something like this diagram.

This allows you to mock up a TestExplorerView and test the logic that now lies in the controller.

Answer 2

Comments

Comments which describe what the code does should be deleted. The code itself should describe what is done by using meaningful names, like your code already do.

private BindingList<TestExplorerItem> _allTests; // all tests found in solution
private IList<TestExplorerItem> _tests; // tests to execute

General

private void UpdateCompletedTestsLabels()
{
    passedTestsLabel.Text = string.Format("{0} Passed", _tests.Count(item => item.Outcome == TestOutcome.Succeeded.ToString()));
    failedTestsLabel.Text = string.Format("{0} Failed", _tests.Count(item => item.Outcome == TestOutcome.Failed.ToString()));
    inconclusiveTestsLabel.Text = string.Format("{0} Inconclusive", _tests.Count(item => item.Outcome ==
    TestOutcome.Inconclusive.ToString()));
}  

For each check if the condition of the Count() method is true, you call ToString(). This can be improved by also extracting this to a separate method.

private int GetTestCountBy(string testOutcome)
{
    return _tests.Count(item => item.Outcome == testOutcome));
}
private void UpdateCompletedTestsLabels()
{
    passedTestsLabel.Text = string.Format("{0} Passed", GetTestCountBy(TestOutcome.Succeeded.ToString()));
    failedTestsLabel.Text = string.Format("{0} Failed", GetTestCountBy(TestOutcome.Failed.ToString()));
    inconclusiveTestsLabel.Text = string.Format("{0} Inconclusive", GetTestCountBy(TestOutcome.Inconclusive.ToString()));
}  

You should do the same where you call Any() like in UpdateProgress(). Also there it only matters if one of the checked states will be at the end of the list.

• In RunSelectedTestMenuItemClicked() a guard clause would be better.