CODEWITHSUNDEEP
unit-testing
ryeguy
ryeguy

Reputation: 66851

How far should I go with unit testing?

I'm trying to unit test in a personal PHP project like a good little programmer, and I'd like to do it correctly. From what I hear what you're supposed to test is just the public interface of a method, but I was wondering if that would still apply to below.

I have a method that generates a password reset token in the event that a user forgets his or her password. The method returns one of two things: nothing (null) if everything worked fine, or an error code signifying that the user with the specified username doesn't exist.

If I'm only testing the public interface, how can I be sure that the password reset token IS going in the database if the username is valid, and ISN'T going in the database if the username is NOT valid? Should I do queries in my tests to validate this? Or should I just kind of assume that my logic is sound?

Now this method is very simple and this isn't that big of a deal - the problem is that this same situation applies to many other methods. What do you do in database centric unit tests?

Code, for reference if needed:

public function generatePasswordReset($username)
{
    $this->sql='SELECT  id
                FROM    users
                WHERE   username = :username';

    $this->addParam(':username', $username);
    $user=$this->query()->fetch();

    if (!$user)
        return self::$E_USER_DOESNT_EXIST;
    else
    {
        $code=md5(uniqid());
        $this->addParams(array(':uid'        => $user['id'],
                               ':code'       => $code,
                               ':duration'   => 24 //in hours, how long reset is valid
                              ));

        //generate new code, delete old one if present
        $this->sql ='DELETE FROM password_resets WHERE user_id=:uid;';
        $this->sql.="INSERT INTO password_resets (user_id, code, expires)
                     VALUES      (:uid, :code, now() + interval ':duration hours')";

        $this->execute();
    }
}

Upvotes: 6

Views: 547

Answers (8)

Bryan Oakley
Bryan Oakley

Reputation: 385970

Unit tests serve the purpose of verifying that a unit works. If you care to know whether a unit works or not, write a test. It's that simple. Choosing to write a unit test or not shouldn't be based on some chart or rule of thumb. As a professional it's your responsibility to deliver working code, and you can't know if it's working or not unless you test it.

Now, that doesn't mean you write a test for each and every line of code. Nor does it necessarily mean you write a unit test for every single function. Deciding to test or not test a particular unit of work boils down to risk. How willing are you to risk that your piece of untested code gets deployed?

If you're asking yourself "how do I know if this functionality works", the answer is "you don't, until you have repeatable tests that prove it works".

Upvotes: 1

Wim Coenen
Wim Coenen

Reputation: 66733

If your unit tests have side-effects (like changing a database) then they have become integration tests. There is nothing wrong in itself with integration tests; any automated testing is good for the quality of your product. But integration tests have a higher maintenance cost because they are more complex and are easier to break.

The trick is therefore to minimize the code that can only be tested with side effects. Isolate and hide the SQL queries in a separate MyDatabase class which does not contain any business logic. Pass an instance of this object to your business logic code.

Then when you unit test your business logic, you can substitute the MyDatabase object with a mock instance which is not connected to a real database, and which can be used to verify that your business logic code uses the database correctly.

See the documentation of SimpleTest (a php mocking framework) for an example.

Upvotes: 0

JeffP
JeffP

Reputation: 1086

Databases are global variables. Global variables are public interfaces for every unit that uses them. Your test cases must therefore vary inputs not only on the function parameter, but also the database inputs.

Upvotes: 0

p.campbell
p.campbell

Reputation: 100567

The great thing about unit testing, for me at least, is that it shows you where you need to refactor. Using your sample code above, you've basically got four things happening in one method:

//1. get the user from the DB
//2. in a big else, check if user is null
//3. create a array containing the userID, a code, and expiry
//4. delete any existing password resets
//5. create a new password reset

Unit testing is also great because it helps highlight dependencies. This method, as shown above, is dependent on a DB, rather than an object that implements an interface. This method interacts with systems outside its scope, and really could only be tested with an integration test, rather than a unit test. Unit tests are for ensuring the working/correctness of a unit of work.

Consider the Single Responsibility Principle: "Do one thing". It applies to methods as well as classes.

I'd suggest that your generatePasswordReset method should be refactored to:

  • be given a pre-defined existing user object/id. Do all those sanity checks outside of this method. Do one thing.
  • put the password reset code into its own method. That would be a single unit of work that could be tested independently of the SELECT, DELETE and INSERT.
  • Make a new method that could be called OverwriteExistingPwdChangeRequests() which would take care of the DELETE + INSERT.

Upvotes: 6

ire_and_curses
ire_and_curses

Reputation: 70152

The reason this function is more difficult to unit test is because the database update is a side-effect of the function (i.e. there is no explicit return for you to test).

One way of dealing with state updates on remote objects like this is to create a mock object that provides the same interface as the DB (i.e. it looks identical from the perspective of your code). Then in your test you can check the state changes within this mock object and confirm you have received what you should.

Upvotes: 3

William Pursell
William Pursell

Reputation: 212248

Testing the public interface is necessary, but not sufficient. There are many philosophies on how much testing is required, and I can only give my own opinion. Test everything. Literally. You should have a test that verifies that each line of code has been exercised by the test suite. (I only say 'each line' because I'm thinking of C and gcov, and gcov provides line-level granularity. If you have a tool that has finer resolution, use it.) If you can add a chunk of code to your code base without adding a test, the test suite should fail.

Upvotes: 0

Pete Duncanson
Pete Duncanson

Reputation: 3246

You can break it down some more, that function is doing alot which makes testing it a bit tricky, not impossible but tricky. If on the other hand you pulled out some smaller extra functions (getUserByUsername, deletePasswordByUserID, addPasswordByUserId, etc. Then you can test those easily enough once and know they work so you don't have to test them again. This way you test the lower down calls making sure they are sound so you don't have to worry about them further up the chain. Then for this function all you need to do is throw it a user that does not exist and make sure it comes back with a USER_DOESNT_EXIST error then one where a user does exist (this is where you test DB comes in). The inner works have already been exercised else where (hopefully).

Upvotes: 1

djna
djna

Reputation: 55907

In general one might "mock" the object you are invoking, verifying that it receives the expected requests.

In this case I'm not sure how helpful that is, you amost end up writing the same logic twice ... we thought we sent "DELETE from password" etc. Oh look we did!

Hmmm, what did we actually check. If the string was badly formed, we woudln't know!

It may be against the letter of Unit testing law, but I would instead test these side-effects by doing separate queries against the database.

Upvotes: 0

Related Questions