Opened 18 years ago

Closed 18 years ago

Last modified 14 years ago

#117 closed task (fixed)

add CxxTest

Reported by: Jan Wassenberg Owned by: Jan Wassenberg
Priority: Should Have Milestone:
Component: Core engine Keywords: test
Cc: Patch:

Description (last modified by Jan Wassenberg)

Background: using an existing Unit/self-test framework is deemed helpful (see 2006-05-27 meeting) Chiefly based on evaluation, we choose CxxTest as unit testing framework.

Basic usage:

  • self-tests are separate Cpp files that contain only the test code. (splitting is necessary so that cxxtestgen doesn't have to parse the entire codebase)
  • they are stored in a tests subdir of the source directory (simplifies finding the relevant test file for purposes of editing it)
  • a workspace generated by premake enumerates all test files, uses custom build step to generate the actual test code, and then links it all into a separate program that runs all tests. Note: separating test code from the main PS executable has several advantages as documented
  • only running certain tests will be possible (shortens iteration time when only editing one module).

TODO:

  • add CxxTest to SVN
  • generate workspace for the test program (via premake)
  • move all existing tests over to that
  • add squelchNextError feature, use it in string_s test
  • make sure all tests run successfully
  • filtering: modify(/replace/subclass) TestRunner and change its runWorld() so that it filters by the SuiteDescription.testName().

Change History (14)

comment:1 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:2 by Simon Brenner, 18 years ago

Obviously, the tests need to link against engine code. Do we plan on splitting the engine into a library linked against by two projects, or making the test code into a library called by the engine itself?

comment:3 by Jan Wassenberg, 18 years ago

Good question. Philip suggests making game code into a library and having the test project reference that. This would have the small advantage of allowing tests to reside in a separate executable, which unclutters ps.exe and would probably make tests run a tiny bit faster. Those are not very weighty, though; let's think about which way would be easier on the build system (which I am currently struggling with).

Engine is static library: I imagine we'd just bundle major chunks of the source into projects, each set to creating a static library. Fortunately main.cpp was set up with this in mind - we can just create one more project and link together main.cpp and the libraries. The test.exe project would do the same, only for all generated test cpp files and the libraries. This approach is the "nicest" and may end up decreasing build time, but requires lots of work in the build system.

If test code becomes a library called by the engine, then all that would actually need to happen is: add the test cpp files to the main project and compile. (note: the test code will be in ps.exe and must be explicitly run, triggered by cmdline param or something). we could also bundle them into a library first, but I don't see much advantage there except possibly decreasing link time. This would obviously be much easier to implement in the build system.

My conclusion? hm, difficult at 3 AM :P I do believe the "correct" approach (the former, originally proposed by Philip) is the better in the long run. Rebuilding the test after code or a test changes should be quicker because the linker doesn't have to juggle the hundreds of object files - instead, only a few libraries. Also, having the test executable separate is a slight bit nicer (see above).

More opinions?

comment:4 by Philip Taylor, 18 years ago

Building the engine as a static library should, in theory, just require Premake's 'package.kind = "winexe"' to be changed to 'package.kind = "lib"'. (And then I guess remove main.cpp, and add a second winexe project which includes main.cpp and links to the library, and a third which runs the tests instead). There might be more complications in practice, possibly major ones, but none that I can specifically predict.

(I've poked around the Premake scripts a bit getting it to compile all the separate projects and DLLs/EXEs for Atlas, so I'd be happy to look for this bit too if there's any problems with that :-) )

If that were happen, I'd also like to use it as an opportunity to split things up further into static libraries in the future, particularly with the Atlas<->game interface code - it's annoying having everything in one giant project with the need to keep filenames unique and no error messages when you forget. And some simple/isolated components could be built simply and isolatedly (and easily redistributably) as an individual library with their own per-project settings, rather than having the entire standard C and C++ libraries and CStr and SpiderMonkey pulled in through precompiled.h. And perhaps it'd even let VS2005's parallel-project-compilation make things a bit faster. (I just hope there aren't any particular problems with static libraries that I'm failing to know about.)

A possibly related point: I noticed the tests were added as ".../tests/test_X.h", but wouldn't ".../tests/X.cpp" be good instead? They're not header files and shouldn't really be included anywhere (except for the generated test runner which does include them once) so .cpp seems more accurate, and the test_ prefix is redundant in any case if they're being built in a separate project and won't have filename conflicts with the other source files (and will (/should) be referred to by paths relative to the project root, so they won't conflict with #includes), and we know they're tests anyway because they're inside a 'tests' directory.

comment:5 by Jan Wassenberg, 18 years ago

Yeah, the "correct" way really is what should be done. I'll suck it up and do it :)

Building the engine as a static library should .. just require .. 'package.kind = "lib"'.

Roger. But the trouble starts with setting up a project for each of the static libs. Looks like major surgery is needed in premake.lua. Example: stuff like the external dependencies would have (ought) to be split up per project/lib.

(I've poked around the Premake scripts a bit getting it to compile all the separate projects and DLLs/EXEs for Atlas, so I'd be happy to look for this bit too if there's any problems with that :-) )

Thanks! It'd be great if we could look at this together.

I'd also like to use it as an opportunity to split things up further into static libraries in the future

Good idea; all excellent reasons.

Proposed set of static libs: (their source directories to include and external library dependencies)

ENGINE
source
	"ps"
	"ps/scripting"
	"ps/Network"
	"ps/GameSetup"
	"ps/XML"
	"simulation"
	"simulation/scripting"
	"sound"
	"scripting"
	"maths"
	"maths/scripting"
libs
	"spidermonkey"
	"xerces"
	"boost"


GRAPHICS
source
	"graphics"
	"graphics/scripting"
	"renderer"
libs
	"misc" (i.e. OpenGL)
	XXX probably also spidermonkey for graphics/scripting?


INTERNATIONALIZATION (may be useful for other projects, hence separate)
source
	"i18n"


ATLAS
source
	"tools/atlas/GameInterface"
	"tools/atlas/GameInterface/Handlers"


GUI
source
	"gui"
	"gui/scripting"


LOWLEVEL
source
	"lib"
	"lib/sysdep"
	"lib/res"
	"lib/res/file"
	"lib/res/graphics"
	"lib/res/sound"
libs
	"libpng"
	"zlib"
	"openal"
	"vorbis"
	"libjpg"
	"dbghelp"
	"directx"

I noticed the tests were added as ".../tests/test_X.h", but wouldn't ".../tests/X.cpp" be good instead? They're not header files and shouldn't really be included anywhere (except for the generated test runner which does include them once) so .cpp seems more accurate, and the test_ prefix is redundant in any case if they're being built in a separate project and won't have filename conflicts with the other source files (and will (/should) be referred to by paths relative to the project root, so they won't conflict with #includes), and we know they're tests anyway because they're inside a 'tests' directory.

OK, this needs some explaining. cxxtest says tests are to be put in "headers"; the Perl script the generates cpp files from them. Only then are the tests capable of running - cxxtestgen needs to insert glue code that calls each test* function, since they're not registered anywhere. It might be possible to name them *.cpp, but then we'd have to disambiguate the actual test and generated test cpp files.

The names are test_* because I figured it'd be safer - the test project does need to set 0ad include paths, after all (for pulling in the headers for engine code). We wouldn't want conflicts there (which would end up silently causing the test not to be included - ugh). The name is a bit redundant, but it does add some safety, in my humble and paranoid opinion :)

comment:6 by Jan Wassenberg, 18 years ago

Project has been split up into static libraries along the lines proposed above.

comment:7 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:8 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:9 by Jan Wassenberg, 18 years ago

(In [4011]) # work on self-tests add ignore-next-error feature, use it in string_s refs #117

comment:10 by Jan Wassenberg, 18 years ago

(In [4021]) # fix errors in self-tests and the code they test. all now run through. (archive_builder and compression tests are disabled ATM)

also fixed some compile warnings inside cxxtest.

refs #117

comment:11 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:12 by Jan Wassenberg, 18 years ago

Description: modified (diff)

I think we can forget the filtering idea. First, the self-test error messages are printed via debug output, so (currently) the test executable needs to be run from within VS for them to be seen. This makes cmdline params much less useful. Also due to cxxtest design (no capability for filtering/stopping tests in a Listener), we'd have to hack cxxtest itself and pass argv through the testroot->GUIrunner->runWorld, which isn't so nice.

Also, the tests now actually work and are fine for a quick test before committing. If they need to really be run often (e.g. a quick test after every compile), it is easy to disable lengthy tests (simply prepend their test_* method name with something else).

Anyone disagree or want to see the filtering after all?

comment:13 by Jan Wassenberg, 18 years ago

Resolution: fixed
Status: newclosed

(In [4022]) # final self-test fixes; integration complete. reinstate and fix archive_builder and compression tests. file: add dir_delete wdbg_sym: stack trace check is now silent. Win32Gui: fix indentation

closes #117

comment:14 by (none), 14 years ago

Milestone: ASAP

Milestone ASAP deleted

Note: See TracTickets for help on using tickets.