How do I unit test this asynchronous code?

Question

I am working a project that puts a gui to a 3rd party database backup utility. This is my first project I have done where I am writing unit tests. I have been writing almost the entire project using TDD methodology up until now, but I got stumped with this function and just wrote it without TDD. Going back I still don't know how to test it.

Note Ignore the code below and check out "Edit #2" below if you are reading this post for the first time which is a refactored version of this code.

    private void ValidateCustomDbPath()
    {
        if (_validateCustomDbPathTask != null && !_validateCustomDbPathTask.IsCompleted)
        {
            _validateCustomDbPathCancellationTokenSource.Cancel();
        }

        if (string.IsNullOrEmpty(_customDbPath))
        {
            Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Validated);
            Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, "");
            _customDbPathCompany = string.Empty;
            UpdateDefaultBackupPath();
            return;
        }

        _validateCustomDbPathCancellationTokenSource = new CancellationTokenSource();
        var ct = _validateCustomDbPathCancellationTokenSource.Token;

        _validateCustomDbPathTask = Task.Run(async () =>
        {
            Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Validating);
            Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, "");
            try
            {
                if (!_diskUtils.File.Exists(_customDbPath))
                {
                    Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Invalid);
                    Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, "File not found");
                    _customDbPathCompany = string.Empty;
                    UpdateDefaultBackupPath();
                    return;
                }
                using (var conn = _connectionProvider.GetConnection(_customDbPath, false))
                using (var trxn = conn.BeginTransaction())
                {

                    var dbSetup = await _dbSetupRepo.GetAsync(conn, trxn);
                    _customDbPathCompany = dbSetup.Company;
                    Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Validated);
                    Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, "");
                    UpdateDefaultBackupPath();
                }
            }
            catch
            {
                Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Invalid);
                Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, "Error getting company");
                _customDbPathCompany = string.Empty;
                UpdateDefaultBackupPath();
            }
        }, ct);
    }

This method is in a ViewModel for a screen. It gets called in the CustomDbPath property setter after a new value is set. The idea is that I have visual indicators in the gui to show if the path provided is valid and UpdateDefaultBackupPath method updates the suggested backup file name based on information in the selected database.

Explaining what you are seeing here, the first IF block cancels the validation task if one was already running and not finished (I know I have yet to make use of the cancellation token). In the second block if no path is provided (the starting state) I don't want to show an error and no need to validate further. In the task, I first indicate that the field is being validated, then I check if the database file can be found on disk, and finally if found I look for information in the database to be used in naming of the backup file name. I am using MVVM Light which is where the Set method comes from (it implements INotifyPropertyChanged).

Everywhere else so far where I have used tasks I have not had a problem testing. I await the method in question and test the results. This case is different. This is called in property setter which obviously can't follow the async await pattern and I wouldn't want it to anyway since it is possible for the user to change the property again before the first validation sequence has finished. The main things I am interested in testing are CustomDbPathValidation and CustomDbPathValidationMessage values. Did it set to validating before validation. Did it set to validated when it succeeded or invalid when it failed. I am more than happy to rewrite this method in a way that makes it testable I just don't know how. Any ideas?

Edit #2:

In the spirit of the suggestions by @vendettamit to follow SRP I have broken down the functions a bit more. GetCompanyInfoFromDbAsync serves to get the company information from the database when appropriate. GetCompanyInfoAsync determines whether the info is retrieved from the database when possible or not if the database is not found. Those two methods will ultimately be moved out to another class and made public for testing purposes. The class I move them to will be injected in to the class shown here via the constructor.

As to some of the points made by @vendettamit:

"You're setting the path if it's empty(Should not be part of validation method)"

I think you misread the code. I am setting the company to blank if the path is blank. The point of this code is to get the company name and use it to craft a filename.

I'm not sure if GetCompanyInfoAsync would meet your standard of SRP or not but it seems odd to me to try and break it down any more than I already have in this edit.

'UpdateDefaultBackupPath() is called in all paths "Code smell"'

I'm guessing you wrote that before you read my first edit. Upon looking back at the code I came to the same conclusion and had already refactored so it was called once.

"And last I saw your edit i.e. ref _customDbPathValidationMessage God be with you."

While I agree that in general ref is to be used very rarely I feel that it is appropriate here. The Set method is from the MVVM Light ViewModelBase base class that this class derives from. It helps with the INotifyPropertyChanged "pattern". They have functions that don't update your backing fields but when I need to change a backing field and notify I choose to use the Set method to reduce the code required. The first parameter is an Expression that allows you to specify the property for which the notification is raised in a way that the compiler can help you catch typos. The ref param is where you provide the backing field for the property, and the next parameter is the new value to be assigned to the backing field. I could have not used Set and used a different helper method provided in the ViewModelBase to raise the notification and then set the backing field prior to that manually. But why? Why add more code? I don't see what it would achieve.

To the comments about the function depending on the state of previous calls (the Task and TaskCancellationSource) I don't see any way to get around this. I need to be able to fire this off while not having the setter wait on the Task to complete. I can't have it "pausing" after them typing in a letter to the edit box the CustomDbPath field is bound to. When the Backup button is pressed (a command on the VM) I will need to check to see if a task is running and wait for it's completion of it is.

The code in ValidateCustomDbPathAsync is what I am still worried about. I could change it to protected etc and await it in a test, but I am still left with the problem that I don't know how to test that it set it to Validating before it performed the validation because by the time the await has returned the result it is too late to check that. This is ultimately the problem that I have and even after refactoring I see no easy way to do that.

Note - This is getting rather long. Does StackOverflow prefer to retain previous edits or should I remove the first edit to reduce the length of this question?

    public string CustomDbPath
    {
        get { return _customDbPath; }
        set
        {
            if (_customDbPath != value)
            {
                Set(() => CustomDbPath, ref _customDbPath, value);
                ValidateCustomDbPath();
            }
        }
    }

    private void ValidateCustomDbPath()
    {
        if (_validateCustomDbPathCts != null)
        {
            _validateCustomDbPathCts.Cancel();
            _validateCustomDbPathCts.Dispose();
        }

        _validateCustomDbPathCts = new CancellationTokenSource();
        _validateCustomDbPathTask = ValidateCustomDbPathAndUpdateDefaultBackupPathAsync(_validateCustomDbPathCts.Token);
    }

    private async Task ValidateCustomDbPathAndUpdateDefaultBackupPathAsync(CancellationToken ct)
    {
        var companyInfo = await ValidateCustomDbPathAsync(ct);

        _customDbPathCompany = companyInfo.Company;
        UpdateDefaultBackupPath();
    }

    private async Task<CompanyInfo> ValidateCustomDbPathAsync(CancellationToken ct)
    {
        Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Validating);
        Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, string.Empty);

        var companyInfo = await GetCompanyInfoAsync(_customDbPath, ct);

        Set(() => CustomDbPathValidation, ref _customDbPathValidation, companyInfo.Error ? ValidationState.Invalid : ValidationState.Validated);
        Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, companyInfo.ErrorMsg);
        return companyInfo;
    }

    private async Task<CompanyInfo> GetCompanyInfoAsync(string dbPath, CancellationToken ct)
    {
        if (string.IsNullOrEmpty(dbPath))
        {
            return new CompanyInfo
            {
                Company = string.Empty,
                Error = false,
                ErrorMsg = string.Empty
            };
        }

        if (_diskUtils.File.Exists(dbPath))
        {
            return await GetCompanyInfoFromDbAsync(dbPath, ct);
        }

        ct.ThrowIfCancellationRequested();

        return new CompanyInfo
        {
            Company = string.Empty,
            Error = true,
            ErrorMsg = "File not found"
        };
    }

    private async Task<CompanyInfo> GetCompanyInfoFromDbAsync(string dbPath, CancellationToken ct)
    {
        try
        {
            using (var conn = _connectionProvider.GetConnection(dbPath, false))
            using (var trxn = conn.BeginTransaction())
            {
                ct.ThrowIfCancellationRequested();

                var dbSetup = await _dbSetupRepo.GetAsync(conn, trxn);

                ct.ThrowIfCancellationRequested();

                return new CompanyInfo
                {
                    Company = dbSetup.Company,
                    Error = false,
                    ErrorMsg = string.Empty
                };
            }
        }
        catch (OperationCanceledException)
        {
            throw;
        }
        catch
        {
            return new CompanyInfo
            {
                Company = string.Empty,
                Error = true,
                ErrorMsg = "Error getting company"
            };
        }
    }

Edit #1:

From some of the suggestion made by @BerinLoritsch, I have rewritten the method as async and broke out some of the logic in to another method which can later be put in to another class and faked during testing. I had to add a pragma statement to get the compiler to quit warning me that I wasn't awaiting the async method (which you can't do in a property settter). I think after rewriting it now better shows the problem I have which I may have not been clear enough on before. I knew I could write this as async and await it and I can test whether it was properly marked as invalid or validated. My problem is, how do I test whether it first marks it as validating before performing the validation? It does this right before performing the validation but once you await the result of this function it is basically too late because the result you get will either be invalid or validated. I'm not sure exactly how to test that. In my mind I'm thinking there might be a way to fake out the newly revised GetCompanyInfoAsync method to return a task that "stalls out" in the test until I want it to complete. If I can control it's completion timing then maybe I can test the ViewModel state before it's completion.

public string CustomDbPath
    {
        get { return _customDbPath; }
        set
        {
            if (_customDbPath != value)
            {
                Set(() => CustomDbPath, ref _customDbPath, value);
                #pragma warning disable 4014
                ValidateCustomDbPathAsync();
                #pragma warning restore 4014
            }
        }
    }

    private async Task ValidateCustomDbPathAsync()
    {
        if (_validateCustomDbPathTask != null && !_validateCustomDbPathTask.IsCompleted)
        {
            _validateCustomDbPathCancellationTokenSource.Cancel();
        }

        _validateCustomDbPathCancellationTokenSource = new CancellationTokenSource();
        var ct = _validateCustomDbPathCancellationTokenSource.Token;

        _validateCustomDbPathTask = Task.Run(async () =>
        {
            Set(() => CustomDbPathValidation, ref _customDbPathValidation, ValidationState.Validating);
            Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, string.Empty);

            var companyInfo = await GetCompanyInfoAsync(_customDbPath, ct);

            Set(() => CustomDbPathValidation, ref _customDbPathValidation, companyInfo.Error ? ValidationState.Invalid : ValidationState.Validated);
            Set(() => CustomDbPathValidationMessage, ref _customDbPathValidationMessage, companyInfo.ErrorMsg);

            _customDbPathCompany = companyInfo.Company;
            UpdateDefaultBackupPath();
        }, ct);

        await _validateCustomDbPathTask;
    }

    private async Task<CompanyInfo> GetCompanyInfoAsync(string dbPath, CancellationToken ct)
    {
        if (string.IsNullOrEmpty(dbPath))
        {
            return new CompanyInfo
            {
                Company = string.Empty,
                Error = false,
                ErrorMsg = string.Empty
            };
        }

        if (!_diskUtils.File.Exists(dbPath))
        {
            return new CompanyInfo
            {
                Company = string.Empty,
                Error = true,
                ErrorMsg = "File not found"
            };
        }

        try
        {
            using (var conn = _connectionProvider.GetConnection(dbPath, false))
            using (var trxn = conn.BeginTransaction())
            {
                var dbSetup = await _dbSetupRepo.GetAsync(conn, trxn);
                return new CompanyInfo
                {
                    Company = dbSetup.Company,
                    Error = false,
                    ErrorMsg = string.Empty
                };
            }
        }
        catch
        {
            return new CompanyInfo
            {
                Company = string.Empty,
                Error = true,
                ErrorMsg = "Error getting company"
            };
        }
    }

Answer 1

With your initial refactoring, you've taken a good step towards being able to unit test these functions. Now you have to work with the balance of accessibility vs. testability. In short, you probably don't want those methods to be called by any code you want, but you do want it called by your unit tests.

If you change the private to protected then you have the option to extend your class and expose the protected methods. If you change the private to internal, then as long as the unit tests are in the same namespace (and assembly if the assembly is sealed), they will be able to access the code. Or you can make them public and everything can access it.

The unit test will look something like this:

 [Test]
 public async Task TestMyDbCode()
 {
     string dbPath = "path/to/db";
     // do your set up

     CompanyInfo info = await myObject.GetCompanyInfoAsync(dbPath, CancellationToken.None);

     Assert.That(info, Is.NotNull());
     // and finish your assertions.
 }

The idea is to break down the tests so that the smallest unit is stable and the code that depends on it can be equally predictable.

You've made the property setter so trivial that it really isn't worth testing as long as the validation code is already well tested.

---

You have a couple options, but in this case the least intrusive way is to declare the ValidateCustomDbPath() as async returning a task. It would look something like this:

private async Task ValidateCustomDbPath()
{
    // prep work

    _validateCustomDbPathTask = Task.Run(async () =>
    {
        // all your DB stuff
    });

    await _validateCustomDbPathTask;
}

By making async code async all the way to the place where you can invoke it, then you already have a mechanism to await for it to be done in the unit test. There are some other options as well, which can be useful if ValidateCustomDbPath must be an Action, or itself is invoked from an event.

The approach requires you to have two methods:

• The async method returning the awaitable return type (usually Task)
• The wrapper method that simply invokes the async method

The advantage is that you can spend your unit testing time on the actual async method, simply ignoring the callback method.

The problem with async void methods is that while you can await other async methods, you can't await the one that returns void, which is why you would need 2 methods if you intend to test the code.

A final option is to put your DB stuff in its own async Task method that you can call directly from your unit test. You can test the paths of the set up method that don't actually do DB work separate from the DB work itself. The important thing is that you expose an async Task method that your unit tests can take advantage of and actually wait for the work to be done.

Answer 2

First, You should really refactor this method. I see a lot of things happening in a single unit which is why you are facing problem writing tests for it. Lets do a little RCA(Root cause analysis)

Few violations of SRP,

• _validateCustomDbPathTask is class variable so this method's output is depending on state of the object.
• You're checking if Task is already running(validating state)
• You're setting the path if it's empty(Should not be part of validation method)
• In the Task you're again validating the path if it exists on disk
• UpdateDefaultBackupPath() is called in all paths "Code smell"
• .. And last I saw your edit i.e. ref _customDbPathValidationMessage God be with you.

Second, After looking at this method it looks like you need to work more on writing unit tests.

Before you write unit tests, write a good testable code.

Although Unit testing promotes Refactoring, So first you shall refactor your method then start writing the tests you might solve it by your own.

Hint - Refactor it(Remove async code), Write tests as it's a sync method. Then change it to Async and fix your tests.