Long test cases

Discussion of testing theory and practice, including methodologies (such as TDD, BDD, DDD, Agile, XP) and software - anything to do with testing goes here. (Formerly "The Testing Side of Development")

Moderator: General Moderators

Post Reply
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Long test cases

Post by Ambush Commander »

Is there anything inherently wrong with long test cases? Is 53 lines excessive?

I ask this because I am currently testing a class that changes an internal state that should be opaque to the user. I can't be bothered to manually create the internal state, so I utilize the functions in the class being currently tested to further test other functions, e.g. if I use add(), the behavior of get() will change accordingly.

This means that if one method in the chain stops working, we'll have a cascading failure down the test case, but it might be a reasonable tradeoff. What do you guys think?
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Be as specific and concise as you can with tests.

If get() broke, you would need to debug to differentiate it from add().
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

I was afraid of that.

Here's a sample snippet:

Code: Select all

function test() {
        // part 1:
        $cache = new HTMLPurifier_DefinitionCache_Serializer('Test');
        
        $config_array = array('Foo' => 'Bar');
        $config_md5   = md5(serialize($config_array));
        
        $file = realpath(
            $rel_file = dirname(__FILE__) .
            '/../../../library/HTMLPurifier/DefinitionCache/Serializer/Test/' .
            $config_md5 . '.ser'
        );
        if($file) unlink($file); // prevent previous failures from causing problems
        
        $config = $this->generateConfigMock($config_array);
        $this->assertIdentical($config_md5, $cache->generateKey($config));
        // part 2
        $def_original = $this->generateDefinition();
        
        $cache->add($def_original, $config);
        $this->assertFileExist($rel_file);
        
        $file_generated = $cache->generateFilePath($config);
        $this->assertIdentical(realpath($rel_file), realpath($file_generated));
        
        $def_1 = $cache->get($config);
        $this->assertIdentical($def_original, $def_1);
        // ...
Part 1 is the initialization stage: the mocks and the subject of the test get instantiated and have their variables stuffed in them. I've already created some convenience functions to help out. Part 2 is the actual test. I don't really see any way I could (without reimplementing the functionality of the class in the test case) separate the test of add() from the test of get(). I have no meaningful way of determining that the contents of $rel_file are what they should be without getting the file contents and unserializing it, which is precisely what get() does.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

I'd test both those with the following:
  • Test the 'add' by using it to write the file, then use:

    Code: Select all

    $this->assertEqual(file_get_contents($cacheFile), "replace this string with the expected contents of the file");
  • and vice versa for the get() - use a file with predetermined contents of a serialized file, then compare the items.
Instantiating the object many times in one testcase is common. Each testcase method is one complete test.
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

That sounds like it would be a very fragile test, in that if you changed any of the variables in the object the slightest, you'd have to recompile the pre-calculated serial. Not to mention that the method for serializing the final result ought to be tested in some way too... how do we combat that?
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Is there anything inherently wrong with long test cases? Is 53 lines excessive?
Probably not. If it does bother you perhaps you could identify where you need a new case by where you need a new setUp() method. If you say all setup code must go in setUp() and you can only have one per test case then it'll force you do use several smaller cases. I don't do that thought. One case per class for me, always. hmmm per the class you are testing is doing too much.... at 53 test case lines though, I doubt it.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Ambush Commander wrote:That sounds like it would be a very fragile test, in that if you changed any of the variables in the object the slightest, you'd have to recompile the pre-calculated serial. Not to mention that the method for serializing the final result ought to be tested in some way too... how do we combat that?
But of course.. if you change your test data, expect different results. To add to that, it's a good thing your testcase is that fragile - you don't want to allow for any margin in behaviour when testing. However, in this situation you can refer to the post which is like minded in the other thread - viewtopic.php?p=384115#384115

To test the serialize method, you simply perform a straight comparison of a "simple" variable:

Code: Select all

function testSerialize ()
{
    $foo = 'bar';
    $cache =& new Cache;
    $this->assertEqual($cache->serialize($foo), 's:3:"bar"');
}
Also to touch on the fragility - this is a perfect indicator of where some refactoring could come in. Perhaps you'll need a different object for reading a file? If so, easy one to mock - thus your test is independent of a file reader, and you can setup another testcase for the file reading class.
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

Also to touch on the fragility - this is a perfect indicator of where some refactoring could come in. Perhaps you'll need a different object for reading a file? If so, easy one to mock - thus your test is independent of a file reader, and you can setup another testcase for the file reading class.
Nooooo!!! (Runs away screaming).

I really don't want to have to build an abstraction layer of the filesystem. Well, it would probably be a good idea, but it's been bouncing around my head for the longest time created a custom "Filesystem" mock that maintains its own "virtual filesystem". Instead of:

Code: Select all

$filesystem->expectOnce('get', array('foo.txt'));
$filesystem->setReturnValue('get', 'blah blah', array('foo.txt'));
You'd be able to write:

Code: Select all

$filesystem->put('foo.txt', 'blah blah');
$filesystem->expectGet('foo.txt');
I'm probably dreaming though.

Yeah, I'm probably going to have to create a Filesystem set of objects and mock them the regular way. I didn't want to: HTML Purifier isn't supposed to be writing the filesystem. Ah well...

Oh, and I'm going to have a fun time injecting the filesystem class into the cache. Probably an optional parameter, methinks.
But of course.. if you change your test data, expect different results. To add to that, it's a good thing your testcase is that fragile - you don't want to allow for any margin in behaviour when testing. However, in this situation you can refer to the post which is like minded in the other thread - viewtopic.php?p=384115#384115
I think I'll use the solution you presented in the other thread. Do I have to test the serializer though? ::is being lazy::
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Nooooo!!! (Runs away screaming).

I really don't want to have to build an abstraction layer of the filesystem.
Hehe lol... no it probably is a good idea. Otherwise you end up reading, writing, creatnig and deleting files in your test cases. I've done that before and its not much fun. I felt like I wanted a test case for my test case. On a similar note I think I would my favourite class of all time today:

Code: Select all

class Shell
{
    public function run($command)
    {
        return `$command`;
    }
}
Short isn't it? xD

This was necessary for the purposes of mocking. I wanted to test the correct command was being executed and the command to I wanted to execute depended on the operating system...I think you might be able to guess what I did next....

Code: Select all

class Server
{
    public function isWinOs()
    {
        return preg_match('~^WIN~i', PHP_OS);
    }
}
OK that class already existed and has a couple of other methods already but you get the point. :)
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

If you decide to abstract and make a virtual filesystem.. viewtopic.php?t=65170

Different naming, but same ballgame.
Post Reply