C++ Unit Testing - Case Study

From Second Life Wiki
Revision as of 16:59, 25 June 2009 by Poppy Linden (talk | contribs)
Jump to navigation Jump to search

A step by step adventure in adding unit tests to code in newview. The sources described here are in the branch http-texture-7 (and possibly trunk by the time you read this!)

Factoring Your Code

The task was to modify the map in the viewer to use the new S3 located multiresolution tile hierarchy to display the world map. There's already code to display the map located in llworldmap.cpp and llworldmapview.cpp that we happily hacked through to implement the new tile pulling and rendering code.

Now, unit tests are associated with cpp files on a 1:1 basis and, since llworldmap.cpp had no unit tests, we could have written a new llworldmap_test.cpp file.

Problem is, when writing unit tests, llworldmap.cpp gets compiled and linked along llworldmap_test.cpp in its own independent llworldmap_test project. That brings in a whole tangle of dependencies that needs to be cleared up, calls to be stubbed, etc... A discouraging task.

So, since we wrote a specific class to handle the new data structure, we choose to isolate that class in its own llworldmipmap.cpp file.

The moral of the story here is that, when writing unit tests for legacy code, you'll have to think before hand at to which set of classes you want to rework and, possibly, refactor and isolate somewhere. It's all for the goodness of the code structure for sure but that's work and you need to consider it carefully before rushing and write unit tests.

Adding the Test Structure

Since CMake is in the way, the first thing we did was to build the minimum scaffolding so that we could rerun CMake, rebuilt the solution and get something clean to start coding.

Create the Test File

The code to be unit tested being in indra/newview/llworldmipmap.cpp, we created a indra/newview/tests/llworldmipmap_test.cpp file. We had to create the tests folder as no unit tests had been written for newview yet.

As said before, do not try to be creative here: the location of the tests folder, its name (tests) and the name of the unit test file cannot depart from a rigid set of rules assumed in LLAddBuildTest.cmake. Of course, changing those rules is possible but it will make all other tests fail or not being build at all so don't even think about doing this... or at least, discuss it on all relevant mailing lists before hand.

Now, we put a very short piece of code in llworldmipmap_test.cpp. Besides the usual license comment header, here's the minimum code so that tut is getting compiled.

// Tut header
#include "../test/lltut.h"

// -------------------------------------------------------------------------------------------
// TUT
// -------------------------------------------------------------------------------------------
namespace tut
{
	// Test wrapper declaration : wrapping nothing for the moment
	struct worldmipmap_test
	{
	};

	// Tut templating thingamagic: test group, object and test instance
	typedef test_group<worldmipmap_test> worldmipmap_t;
	typedef worldmipmap_t::object worldmipmap_object_t;
	tut::worldmipmap_t tut_worldmipmap("worldmipmap");

	// ---------------------------------------------------------------------------------------
	// Test functions : test doing nothing for the moment
	// ---------------------------------------------------------------------------------------
	template<> template<>
	void worldmap_object_t::test<1>()
	{
	}
}

We're done with the test file creation.

CMakeList.txt

Now we need to instruct CMake that there's some new unit tests to build and create the projects for it. This is done editing indra/newview/CMakeList.txt. We added the following at the end of the file:

# Add tests
include(LLAddBuildTest)
set(viewer_TEST_SOURCE_FILES
  llworldmipmap.cpp
  )
LL_ADD_PROJECT_UNIT_TESTS(viewer "${viewer_TEST_SOURCE_FILES}")

We have to include the LLAddBuildTest cmake file here to get the LL_ADD_PROJECT_UNIT_TESTS macro.

Our code (like most newview code) is depending heavily on the precompiled header. We need to add llviewerprecompiledheaders.cpp to the test project which will prevent lots of link errors (at least, for anything related to the precompiled header).

# Add tests
include(LLAddBuildTest)
set(viewer_TEST_SOURCE_FILES
  llworldmipmap.cpp
  )
set_source_files_properties(
  ${viewer_TEST_SOURCE_FILES}
  PROPERTIES
    LL_TEST_ADDITIONAL_SOURCE_FILES llviewerprecompiledheaders.cpp
  )
LL_ADD_PROJECT_UNIT_TESTS(viewer "${viewer_TEST_SOURCE_FILES}")

The arguments of the macro are:

  • viewer : the target project this unit test must be added to as a dependency
  • "${viewer_TEST_SOURCE_FILES}" : the list of cpp files to test

OK, now we're ready to launch CMake again and rebuild the solution.

Rebuild the Solution

We built the solution as usual. We always delete the indra/build-platform folder and type the following command (after navigating to the indra folder):

./develop.py

Nothing unusual here.

Now, we open the Second Life project and verify that the projects PROJECT_viewer_TEST_llworldmipmap and viewer_tests are in the list of projects. Open the PROJECT_viewer_TEST_llworldmipmap project and verify that the following files are in the list:

      lltut.cpp
      llviewerprecompiledheaders.cpp
      llworldmipmap.cpp
      llworldmipmap_test.cpp
      test.cpp

Build the solution... Surprise: link errors:

[...]
1>llworldmipmap.obj : error LNK2019: unresolved external symbol "public: void __thiscall LLViewerImage::setBoostLevel(int)" (?setBoostLevel@LLViewerImage@@QAEXH@Z) referenced in function "public: void __thiscall LLWorldMipmap::equalizeBoostLevels(void)" (?equalizeBoostLevels@LLWorldMipmap@@QAEXXZ)
1>llworldmipmap.obj : error LNK2019: unresolved external symbol "public: class LLViewerImage * __thiscall LLViewerImageList::getImageFromUrl(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &,int,int,int,unsigned int,class LLUUID const &)" (?getImageFromUrl@LLViewerImageList@@QAEPAVLLViewerImage@@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@HHHIABVLLUUID@@@Z) referenced in function "private: class LLPointer<class LLViewerImage> __thiscall LLWorldMipmap::loadObjectsTile(unsigned int,unsigned int,int)" (?loadObjectsTile@LLWorldMipmap@@AAE?AV?$LLPointer@VLLViewerImage@@@@IIH@Z)
1>llworldmipmap.obj : error LNK2001: unresolved external symbol "class LLViewerImageList gImageList" (?gImageList@@3VLLViewerImageList@@A)
1>RelWithDebInfo\llworldmipmap_test.exe : fatal error LNK1120: 3 unresolved externals
1>Build log was saved at "file://c:\Develop\http-texture-7\indra\build-vc80\newview\llworldmipmap_test.dir\RelWithDebInfo\BuildLog.htm"
1>llworldmipmap_test - 4 error(s), 0 warning(s)
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

We're not done with prep work: even if we are not doing anything with LLWorldMipmap objects yet, we're still compiling llworldmipmap.cpp and linking its object code. Now it's time to dive into writing code.

Writing the Test File

Stubbing Classes

The first thing we wanted to do was stubbing the code so that we could link. In our case, since the dependencies were well isolated, we just let the linker guide us listing the symbols it couldn't resolve and wrote stub implementation for them. Here's the code we added at the beginning of llworldmipmap_test.cpp:

// Precompiled header: almost always required for newview cpp files
#include "../llviewerprecompiledheaders.h"
// Class to test
#include "../llworldmipmap.h"
// Dependencies
#include "../llviewerimagelist.h"
// Tut header
#include "../test/lltut.h"

// -------------------------------------------------------------------------------------------
// Stubbing: Declarations required to link and run the class being tested
// Notes: 
// * Add here stubbed implementation of the few classes and methods used in the class to be tested
// * Add as little as possible (let the link errors guide you)
// * Do not make any assumption as to how those classes or methods work (i.e. don't copy/paste code)
// * A simulator for a class can be implemented here. Please comment and document thoroughly.

LLViewerImageList::LLViewerImageList() { }
LLViewerImageList::~LLViewerImageList() { }

LLViewerImageList gImageList;

LLViewerImage* LLViewerImageList::getImageFromUrl(const std::string& url,
                                                  BOOL usemipmaps,
                                                  BOOL level_immediate,
                                                  LLGLint internal_format,
                                                  LLGLenum primary_format, 
                                                  const LLUUID& force_id)
{ return NULL; }
void LLViewerImage::setBoostLevel(S32 level) { }

// End Stubbing
// -------------------------------------------------------------------------------------------

Now, building only the PROJECT_viewer_TEST_llworldmipmap project, we get:

1>------ Build started: Project: llworldmipmap_test, Configuration: RelWithDebInfo Win32 ------
1>Compiling...
1>llworldmipmap_test.cpp
1>Linking...
1>Embedding manifest...
1>Build log was saved at "file://c:\Develop\http-texture-7\indra\build-vc80\newview\llworldmipmap_test.dir\RelWithDebInfo\BuildLog.htm"
1>llworldmipmap_test - 0 error(s), 0 warning(s)
========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

Great! We can now run the tests (which actually includes a single test doing nothing) and see what happens. Select the viewer_tests project and build it. Here's the result in the Output tab:

1>------ Build started: Project: llworldmipmap_test_ok, Configuration: RelWithDebInfo Win32 ------
1>Generating llworldmipmap_test_ok.txt
1>Total Tests:   1
1>Passed Tests: 1
1>Total Tests:   1
1>Passed Tests: 1
1>Build log was saved at "file://c:\Develop\http-texture-7\indra\build-vc80\newview\llworldmipmap_test_ok.dir\RelWithDebInfo\BuildLog.htm"
1>llworldmipmap_test_ok - 0 error(s), 0 warning(s)
========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

Success! We're now ready to add some real unit tests.

Writing the Test Cases

First, we need to complete the wrapper class so that it wraps the class to be tested:

	// Test wrapper declaration
	struct worldmipmap_test
	{
		// Derived test class
		class LLTestWorldMipmap : public LLWorldMipmap
		{
			// Put here stubbs of virtual methods we shouldn't call all the way down
		};
		// Instance to be tested
		LLTestWorldMipmap* mMap;

		// Constructor and destructor of the test wrapper
		worldmipmap_test()
		{
			mMap = new LLTestWorldMipmap;
		}
		~worldmipmap_test()
		{
			delete mMap;
		}
	};

There are actually other ways of writing unit tests. It's not required to write a wrapper class but we found it easier to follow and, potentially, a great trick to overload a couple of methods that could be impossible to test.

Writing the tests is the important and, actually, easiest part. The trick is to know what to test and how. In the case of llworldmipmap, we simply took the list of all public methods and wrote a test for each:

	// ---------------------------------------------------------------------------------------
	// Test functions
	// Notes:
	// * Test as many as you possibly can without requiring a full blown simulation of everything
	// * The tests are executed in sequence so the test instance state may change between calls
	// * Remember that you cannot test private methods with tut
	// ---------------------------------------------------------------------------------------
	// Test static methods
	// Test 1 : scaleToLevel()
	template<> template<>
	void worldmipmap_object_t::test<1>()
	{
		S32 level = mMap->scaleToLevel(0.0);
		ensure("scaleToLevel() test 1 failed", level == LLWorldMipmap::MAP_LEVELS);
		level = mMap->scaleToLevel(LLWorldMipmap::MAP_TILE_SIZE);
		ensure("scaleToLevel() test 2 failed", level == 1);
		level = mMap->scaleToLevel(10 * LLWorldMipmap::MAP_TILE_SIZE);
		ensure("scaleToLevel() test 3 failed", level == 1);
	}
	// Test 2 : globalToMipmap()
	template<> template<>
	void worldmipmap_object_t::test<2>()
	{
		U32 grid_x, grid_y;
		mMap->globalToMipmap(1000.f*REGION_WIDTH_METERS, 1000.f*REGION_WIDTH_METERS, 1, &grid_x, &grid_y);
		ensure("globalToMipmap() test 1 failed", (grid_x == 1000) && (grid_y == 1000));
		mMap->globalToMipmap(0.0, 0.0, LLWorldMipmap::MAP_TILE_SIZE, &grid_x, &grid_y);
		ensure("globalToMipmap() test 2 failed", (grid_x == 0) && (grid_y == 0));
	}
	// Test 3 : getObjectsTile()
	template<> template<>
	void worldmipmap_object_t::test<3>()
	{
		// Depends on some inline methods in LLViewerImage... Thinking about how to make this work
		// LLPointer<LLViewerImage> img = mMap->getObjectsTile(0, 0, 1);
		// ensure("getObjectsTile() test failed", img.isNull());
	}
	// Test 4 : equalizeBoostLevels()
	template<> template<>
	void worldmipmap_object_t::test<4>()
	{
		try
		{
			mMap->equalizeBoostLevels();
		} 
		catch (...)
		{
			fail("equalizeBoostLevels() test failed");
		}
	}
	// Test 5 : dropBoostLevels()
	template<> template<>
	void worldmipmap_object_t::test<5>()
	{
		try
		{
			mMap->dropBoostLevels();
		} 
		catch (...)
		{
			fail("dropBoostLevels() test failed");
		}
	}
	// Test 6 : reset()
	template<> template<>
	void worldmipmap_object_t::test<6>()
	{
		try
		{
			mMap->reset();
		} 
		catch (...)
		{
			fail("reset() test failed");
		}
	}

Committing the Test

There's nothing special to do to get the unit tests added to the list of tests being run. Just commit the code as you usually do. Don't forget to add the new files and folder, and commit the CMakeList.txt changes.