Monday, December 17, 2018

Unit test green and no sleep!

When you have a unit test that depends on the outcome of a slow process, you could use Thread.Sleep to ensure the test is asserted after the process finishes. However this approach will make your test consistently much slower than the system under test.
You can use a delegate in another thread to make a much faster test, but only if you avoid the little pitfall I had today.

The system under test I was working with revolved around a FileSystemWatcher object.  By subscribing to the Created event of this class, you can react to the addition of a new file to a folder. In this case, the name of the new file should be added to a list to be viewed by the user.


using System;
using System.Collections.ObjectModel;
using System.IO;

namespace PDFViewer
{
    /// <summary>
    /// monitors input folder for files,
    /// adds new files to the list and removes read ones if list is over ten items long
    /// </summary>
    internal class ListManager
    {
        internal FileSystemWatcher FileSystemWatcher;
        internal ObservableCollection<ListItem> ListOfItems { get; set; }

        public ListManager(string folderToManage)
        {
            if (Directory.Exists(folderToManage)  == false)
            {
                throw new DirectoryNotFoundException("Listmanager must have existing folder with read permission");
            }

            ListOfItems = new ObservableCollection<ListItem>();
            FileSystemWatcher = new FileSystemWatcher(folderToManage, "*.pdf");
            FileSystemWatcher.Created += OnPdfCreated;
            FileSystemWatcher.Deleted += OnPdfDeleted;
            FileSystemWatcher.EnableRaisingEvents = true;
        }

        private void OnPdfDeleted(object sender, FileSystemEventArgs e)
        {
            throw new NotImplementedException();
        }

        public void OnPdfCreated(object item, FileSystemEventArgs e)
        {
            ListOfItems.Add(new ListItem(e.Name));
        }
    }
}


I wanted to test that I correctly understood how it worked. My first test was straightforward:
[TestMethod]
        public void ManagerAddsNewPdfToList()
        {
            //create temporary filename
            var folder = Path.GetTempPath();
            var file = Path.GetTempFileName() + ".pdf";
            var path = Path.Combine(folder, file);

            //start watching target folder
            var manager = new ListManager(folder);

            File.Create(path);

            //If a file is created, manager should add its name to the list
            Assert.IsTrue(manager.ListOfItems.Count == 1);

            //cleanup
            File.Delete(path);
        }

This test failed. Puzzled I stepped through it and I found that the OnPdfCreated was called AFTER the test was asserted. So the test was going to fast for the file operation to complete.
After poking around the internet a little I found how to write a better test using an AutoResetEvent:
- The event is non-signalled, (stopped).
- When a file is created the event is signalled.
- The test checks if the event was signalled or a timeout happened.

Testing, take two:

        [TestMethod]
        public void ManagerAddsNewPdfToList()
        {
            var creation = new AutoResetEvent(false);

            //Create temporary filename
            var folder = Path.GetTempPath();
            var file = Path.GetTempFileName() + ".pdf";
            var path = Path.Combine(folder, file);

            //Start watching target folder
            var manager = new ListManager(folder);
            
            //Subscribe a delegate to the Created event to set creation to signalled.
            manager.FileSystemWatcher.Created += (s, e) => { creation.Set(); };

            File.Create(path);

            
            //Verify that the Created event is fired.
            var eventFired = creation.WaitOne(1000);
            Assert.IsTrue(eventFired, "Event did not fire.");
            
            //If a file is created, manager should add its name to the list
            //Assert.IsTrue(manager.ListOfItems.Count == 1, "List count is not 1.");

            //Cleanup
            File.Delete(path);
        }

So this should work like a charm, right?
Wrong!

Message: Test method WPFViewerTest.FileManagerTest.ManagerAddsNewPdfToList threw exception: 
System.IO.IOException: The process cannot access the file 
'C:\Users\<snip>\AppData\Local\Temp\tmp404.tmp.pdf' because it is being used by another process.

Again, this error was not consistently reproducible by stepping through code. Sometimes I would get an exception, sometimes not. I do not quite understand how adding an AutoResetEvent, which does not itself operate on the file, can cause this problem.

The solution though proved remarkable simple:
replace File.Create(path); with

//Without using, you get an IO exception
using (File.Create(path))
    { }

This ensures that the file handle is closed after the file is created, and the test passes.