C# Unit Testing – What Kind of Unit Tests Should Be Written for a Call to a Database Update Function?

csqlunit testing

I'm dealing with a lot of functions like the one below and I'm unsure of what kind of unit tests should be written for them.

public void UpdateEmployeeClockIn(int employeeId)
{
var sql = string.format("Update Employee set ClockIn = GETUTCDATE() where employeeId={0}", employeeId);

db.ExecuteNonQuery(sql);
}

I'm thinking I could refactor this method into one that maybe returns a sql statement and I could run a few tests on the sql statement to make sure there is a correct where clause etc. Any ideas? I keep running into code like this where I simply do not know how or what to test for.

Best Answer

It depends on the layer of abstraction between this code and the database.

If an abstraction there is, and you can actually put a mock instead of the object which actually accesses a real database, then your unit tests may check that the correct SQL query is sent to the mock when the method is called with an arbitrary argument, say 123. Given the actual code and the fact that unit testing is white box testing, there is nothing else here to test. Things could be different if you were sanitizing your inputs, for instance by checking that employeeId cannot be negative or zero.

If there are no abstractions, you can't write unit tests for this method without refactoring it first. You can write automated tests, but those would be integration, system, functional or acceptance tests, not unit tests. In this case, you'll run:

  • A similar test which passes an ID of an existing employee, and checks that the database entry was changed.

  • A test which uses an ID which doesn't exist.

  • A test which runs on a database set in read-only mode, or an unreachable database (both cases are especially important if you have a cluster).

Note that:

  • While in the actual piece of code, SQL Injection is impossible, this is still a wrong way to use the database. When you use parametrized queries, the database engine can cache the execution plan. In your case, you are virtually preventing it from doing this.

  • You're not sanitizing your inputs. This could be intentional, if you want to let your database return an error on invalid values, but usually, it indicates that the developer simply forgot to do it. This could make it easier to perform DOS attacks against your database.

  • You are not catching any exception. Is it intentional? It may be: for instance, you may prefer passing the exception up the stack in order for the business layer or presentation layer to deal with it. However, there are cases, where you have to deal with some exceptions right now. For instance, if accessing a cluster, you may have to deal with read-only instances or plain connection loss; instead of simply showing the error to the user, the method could handle the exception gracefully by retrying the query or doing what makes sense to do in a specific situation.

Related Topic