Enable testing via CMake, probably using CTest

Bug #544036 reported by Jens Beyer
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Medium
Unassigned

Bug Description

Automatic codetesting needs to be enabled. This was enabled as default in SCons.
Check if CTest can be used for that, otherwise do it manually.

Related branches

tags: added: buildsystem cmake codecheck
Revision history for this message
SirVer (sirver) wrote :

it would be great if the lua testsuite(s) can be somehow merged into this. It is a quite unconventional way of running tests and it has no standard reporting.

Changed in widelands:
status: New → Confirmed
Changed in widelands:
milestone: none → build16-rc1
Revision history for this message
Jens Beyer (qcumber-some) wrote :

CTest is currently up and running for 1 test (test_widelands_scripting) and could use two more tests (test_economy and test_io_filesystem) if they would compile.

The lua test suite is a different way:

It starts the game and therefor needs a build host capable of a GUI. It should be possible to skip that test for certain circumstances.
The lua test suite needs to be incorporated in CTest.

Changed in widelands:
assignee: nobody → Jens Beyer (Qcumber-some) (qcumber-some)
Revision history for this message
Jens Beyer (qcumber-some) wrote :

I would suggest splitting up this bug into two:

1. Enable testing via CTest (which is working and should be set to Fix Committed)
2. Incorporate Lua Test Suite into CTest, which needs another type of infrastructure, which should be set to "Confirmed" and "Wishlist".

This way, we can get the issue for Build16 'fixed' and can target the Lua Test Suite integration for maybe Build17.

Changed in widelands:
status: Confirmed → In Progress
Revision history for this message
SirVer (sirver) wrote :

I can easily live with that, Jens. +1

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Jens: Sounds reasonable. +1

Revision history for this message
Jari Hautio (jarih) wrote :

I have fixed test_economy and test_io_filesystem to compile and integrated them to CTest in lp:~jarih/widelands/fix-unit-tests. Filesystem tests required some small changes to FileSystem code to get tests to pass. I had to comment out some error condition tests that did not pass and did not not look consistent with other tests.

Should I merge these before build16?

Revision history for this message
SirVer (sirver) wrote :

I am all for the changes you did, but I am against merging this before build 16 if it does not fix some know bug.

That said, this bug is fixed for build 16 then and can be retargeted, or not?

Revision history for this message
Jari Hautio (jarih) wrote :

It does not fix any open bugs, so retargetting is ok. The fix allows relative paths to be used for datadir in linux (like datadir=../, but this can be considered a feature.

Revision history for this message
Nasenbaer (nasenbaer) wrote :

as mentioned above: retargeted

Changed in widelands:
milestone: build16-rc1 → build17-rc1
Revision history for this message
Jens Beyer (qcumber-some) wrote :

Bug #944350 "Incorporate Lua Test Suite into CTest" has been split out.
Now this bug is only about making CTest up and running (which it is, with one test suite).

Now we still have the open (and very stale) branch by jarih to fix test_economy and test_io_filesystem.

I guess it's too late to incorporate that into build17 now, isn't it? Retarget again?

Revision history for this message
Jari Hautio (jarih) wrote :

I Just recently had time update the branch up to current trunk. But I think this should be ok for merging, and I'd like to get it in finally. So I'll see there are any comments on merge request.

While getting the tests to run, I fixed some file system special cases for windows, changed file system code to allow using relative paths on command line, and hard coded windows version to use executable path as default search path - just like in Mac. Earlier this was done using dots in paths during building.

Revision history for this message
SirVer (sirver) wrote : Re: [Bug 544036] Re: Enable testing via CMake, probably using CTest

>I guess it's too late to incorporate that into build17 now, isn't it?
>Retarget again?
Why? There is still time till first now feature freeze.

Jari Hautio (jarih)
Changed in widelands:
status: In Progress → Fix Committed
Changed in widelands:
assignee: Jens Beyer (Qcumber-some) (qcumber-some) → nobody
Revision history for this message
SirVer (sirver) wrote :

Released in build17-rc1.

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.