Unit testing that provides good coverage while avoiding unnecessary tests

Question

I wrote a class that is an enumerable wrapper that caches the results of a base enumerable, only getting the next element if we enumerate and get to the end of the cached results. It can be multithreaded (getting the next item in another thread) or single-threaded (getting the next item in the current thread).

I read on unit-testing and would like my head around the relevant tests. I am using nunit . My main problem is that I have already written my class and am using it. It works for what I use for it (currently one). So, I write my tests, just trying to think about things that may go wrong, which, given that I tested informally, I probably unconsciously write tests that, as I know, have already been tested. How can I get a recording balance between too many / fine tests and too few tests?

• Should I test only public methods / constructors or should I test each method?
• Should I test the CachedStreamingEnumerable.CachedStreamingEnumerator class separately?
• Currently, I am only testing when the class is set to single-threaded. How can I test it with multithreading, given that it may take me a while before the item is fetched and added to the cache?
• What tests are I missing to provide good coverage? I don’t need it anymore?

---

The code for the class and the testing class are 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; } } 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) { } } }); } } 

Answer 1

Ad 1)
If you need to check private methods, this should tell you something; your class may have too many responsibilities. Quite often private methods are separate classes awaiting birth :-)

Ad 2)
Yes

Ad 3)
Following the same arguments as 1, the functioning of streaming should probably not be performed inside the class if it can be avoided. I remember reading about it in Robert Martin's Clean Code. He argues that something like threads is a separate issue that needs to be separated from other areas of business logic.

Announcement 4)
Private methods are the hardest to cover. So, I will turn back to my answer 1. If your private methods were public methods in separate classes, it would be much easier to cover them. In addition, the test of your main class will be easier to understand.

Regards, Morten

Answer 2

Instead of specifying you with details, I just advise you to be practical and follow the “Critical Baby Law” when creating your tests. You do not need to check every accessor or every small piece of standard code.

Think about the things that will inflict the worst on your class and protect them. Check boundary conditions. Use any memories that you have about what might break similar code in your past experience. Try test data data that may be unexpected.

You probably are not doing this as an academic exercise. You probably want to make sure that your class is solid and that it will remain so when you return later to reorganize it, or when you want to make sure that this is not the cause of incorrect behavior in one of its client classes.

Your every test should be there for some reason, and not just because you can be great at the next TDD club meeting!