Unit testing, ensuring good coverage while avoiding unneeded tests

I've written class, which is an enumerable wrapper that caches the results of an underlying enumerable, only getting the next element if we enumerate and reach the end of the cached results. It can be multi-threaded (getting the next item in another thread) or single threaded (getting the next item in the current thread).

I'm reading up on unit-testing and would like to get my head around appropriate tests. I'm using nunit. My main issue is that i've already written my class and am using it. It works for what i'm using it for (one thing currently). So, i'm writing my tests by just trying to think of things that could go wrong, which given that i've tested unofficially i'm probably unconsciously writing tests i know i've already checked. How can i get the write balance between too many/fine-grained tests, and too few tests?

1. Should i only be testing public methods/constructors or should i test every method?
2. Should i test the CachedStreamingEnumerable.CachedStreamingEnumerator class separately?
3. Currently i'm only testing when the class is set to be single-threaded. How do i go about testing it when multi-threaded, given that i might need to wait a period of time before an item is retrieved and added to the cache?
4. What tests am i missing to ensure good coverage? Are any i've already got not needed?

---

Code for the class, and test class below.

CachedStreamingEnumerable

/// <summary>
/// An enumerable that wraps another enumerable where getting the next item is a costly operation.
/// It keeps a cache of items, getting the next item from the underlying enumerable only if we iterate to the end of the cache.
/// </summary>
/// <typeparam name="T">The type that we're enumerating over.</typeparam>
public class CachedStreamingEnumerable<T> : IEnumerable<T>
{
    /// <summary>
    /// An enumerator that wraps another enumerator,
    /// keeping track of whether we got to the end before disposing.
    /// </summary>
    public class CachedStreamingEnumerator : IEnumerator<T>
    {
        public class DisposedEventArgs : EventArgs
        {
            public bool CompletedEnumeration;

            public DisposedEventArgs(bool completedEnumeration)
            {
                CompletedEnumeration = completedEnumeration;
            }开发者_如何学Python
        }

        private IEnumerator<T> _UnderlyingEnumerator;

        private bool _FinishedEnumerating = false;

        // An event for when this enumerator is disposed.
        public event EventHandler<DisposedEventArgs> Disposed;

        public CachedStreamingEnumerator(IEnumerator<T> UnderlyingEnumerator)
        {
            _UnderlyingEnumerator = UnderlyingEnumerator;
        }

        public T Current
        {
            get { return _UnderlyingEnumerator.Current; }
        }

        public void Dispose()
        {
            _UnderlyingEnumerator.Dispose();

            if (Disposed != null)
                Disposed(this, new DisposedEventArgs(_FinishedEnumerating));
        }

        object System.Collections.IEnumerator.Current
        {
            get { return _UnderlyingEnumerator.Current; }
        }

        public bool MoveNext()
        {
            bool MoveNextResult = _UnderlyingEnumerator.MoveNext();

            if (!MoveNextResult)
            {
                _FinishedEnumerating = true;
            }

            return MoveNextResult;
        }

        public void Reset()
        {
            _FinishedEnumerating = false;
            _UnderlyingEnumerator.Reset();
        }
    }

    private bool _MultiThreaded = false;

    // The slow enumerator.
    private IEnumerator<T> _SourceEnumerator;

    // Whether we're currently already getting the next item.
    private bool _GettingNextItem = false;

    // Whether we've got to the end of the source enumerator.
    private bool _EndOfSourceEnumerator = false;

    // The list of values we've got so far.
    private List<T> _CachedValues = new List<T>();

    // An object to lock against, to protect the cached value list.
    private object _CachedValuesLock = new object();

    // A reset event to indicate whether the cached list is safe, or whether we're currently enumerating over it.
    private ManualResetEvent _CachedValuesSafe = new ManualResetEvent(true);
    private int _EnumerationCount = 0;

    /// <summary>
    /// Creates a new instance of CachedStreamingEnumerable.
    /// </summary>
    /// <param name="Source">The enumerable to wrap.</param>
    /// <param name="MultiThreaded">True to load items in another thread, otherwise false.</param>
    public CachedStreamingEnumerable(IEnumerable<T> Source, bool MultiThreaded)
    {
        this._MultiThreaded = MultiThreaded;

        if (Source == null)
        {
            throw new ArgumentNullException("Source");
        }

        _SourceEnumerator = Source.GetEnumerator();
    }

    /// <summary>
    /// Handler for when the enumerator is disposed.
    /// </summary>
    /// <param name="sender"></param>
    /// <param name="e"></param>
    private void Enum_Disposed(object sender,  CachedStreamingEnumerator.DisposedEventArgs e)
    {
        // The cached list is now safe (because we've finished enumerating).
        lock (_CachedValuesLock)
        {
            // Reduce our count of (possible) nested enumerations
            _EnumerationCount--;
            // Pulse the monitor since this could be the last enumeration
            Monitor.Pulse(_CachedValuesLock);
        }

        // If we've got to the end of the enumeration,
        // and our underlying enumeration has more elements,
        // and we're not getting the next item already
        if (e.CompletedEnumeration && !_EndOfSourceEnumerator && !_GettingNextItem)
        {
            _GettingNextItem = true;

            if (_MultiThreaded)
            {
                ThreadPool.QueueUserWorkItem((Arg) =>
                {
                    AddNextItem();
                });
            }
            else
                AddNextItem();
        }
    }

    /// <summary>
    /// Adds the next item from the source enumerator to our list of cached values.
    /// </summary>
    private void AddNextItem()
    {
        if (_SourceEnumerator.MoveNext())
        {
            lock (_CachedValuesLock)
            {
                while (_EnumerationCount != 0)
                {
                    Monitor.Wait(_CachedValuesLock);
                }

                _CachedValues.Add(_SourceEnumerator.Current);
            }
        }
        else
        {
            _EndOfSourceEnumerator = true;
        }

        _GettingNextItem = false;
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }

    public IEnumerator<T> GetEnumerator()
    {
        lock (_CachedValuesLock)
        {
            var Enum = new CachedStreamingEnumerator(_CachedValues.GetEnumerator());

            Enum.Disposed += new EventHandler<CachedStreamingEnumerator.DisposedEventArgs>(Enum_Disposed);

            _EnumerationCount++;

            return Enum;
        }
    }
}

---

CachedStreamingEnumerableTests

[TestFixture]
public class CachedStreamingEnumerableTests
{
    public bool EnumerationsAreSame<T>(IEnumerable<T> first, IEnumerable<T> second)
    {
        if (first.Count() != second.Count())
            return false;

        return !first.Zip(second, (f, s) => !s.Equals(f)).Any(diff => diff);
    }

    [Test]
    public void InstanciatingWithNullParameterThrowsException()
    {
        Assert.Throws<ArgumentNullException>(() => new CachedStreamingEnumerable<int>(null, false));
    }

    [Test]
    public void SameSequenceAsUnderlyingEnumerationOnceCached()
    {
        var SourceEnumerable = Enumerable.Range(0, 10);
        var CachedEnumerable = new CachedStreamingEnumerable<int>(SourceEnumerable, false);

        // Enumerate the cached enumerable completely once for each item, so we ensure we cache all items
        foreach (var x in SourceEnumerable)
        {
            foreach (var i in CachedEnumerable)
            {

            }
        }

        Assert.IsTrue(EnumerationsAreSame(Enumerable.Range(0, 10), CachedEnumerable));
    }

    [Test]
    public void CanNestEnumerations()
    {
        var SourceEnumerable = Enumerable.Range(0, 10).Select(i => (decimal)i);
        var CachedEnumerable = new CachedStreamingEnumerable<decimal>(SourceEnumerable, false);

        Assert.DoesNotThrow(() =>
            {
                foreach (var d in CachedEnumerable)
                {
                    foreach (var d2 in CachedEnumerable)
                    {

                    }
                }
            });
    }
}

---

Ad 1)
If you need to test private methods, this should tell you something; probably that your class has too much responsibilities. Quite often, private methods are separate classes waiting to be born :-)

Ad 2)
Yes

Ad 3)
Following the same argument as 1, threading functionality should probably not be done inside the class if it can be avoided. I recall reading something about this in "Clean Code" by Robert Martin. He states something like that threading is a separate concern, that should be separated from other peaces of business logic.

Ad 4)
The private methods are the hardest to cover. Thus, I again turn to my answer 1. If your private methods were public methods in seperate classes, they would be much easier to cover. Also, the test of your main class would be easier to understand.

Regards, Morten

---

Rather than riddle you with details, I'd simply advise you to be practical and to follow the "Law of the Critical Few" when creating your tests. You do not need to test every accessor or every small fragment of industry-standard code.

Think of what kinds of things would hurt your class the worst and guard against them. Check for boundary conditions. Use any memories you have as to what may have broken similar code in your past experience. Try test data values that may be unexpected.

You are probably not doing this as an academic exercise. You probably want to ensure that your class is solid and that it will stay that way when you go back later to refactor it or when you want to ensure that it is not the cause of misbehavior in one of its client classes.

Your every test should be there for a reason, not just so you can be cool at the next TDD club meeting!