I'm really happy to see this proposal. I agree there has been too many
regressions lately and I definitely agree an automated test suite will help us
discover and fix these faster. I haven't really looked much at the tests
themselves, but it is really easy and straight-forward to run them at least. :)
I cannot really say much about the c++ code in here, but I've looked over the
Python code, and here are some comments:
The first way of loading tests in test_regression.py looks redundant, but I
assume this was where the script started out. Can probably be removed now,
though.
I think we should consider changing test_loader.discover() pattern to something
more like the default one ("test*.py"). Currently it seem to check the __init__
file for test too which is unnecessary, and it could be useful if we want to
add some helper files or similar at some point. It would also allow us to..
Please rename the __maps__ directory. (Maybe just nitpick, but I don't really
see the need for the underscores except to dodge the test file pattern)
Regarding the hardcoded path to the binary, I think it makes the most sense to
provide a command line option to set this, and defaulting to "./widelands" if
not present. This is assuming the common invocation is running
./regression_test.py from the root directory¸ which with the command line
option would look something like this: ./regression_test.py
../some/other/dir/with/widelands
The iteritems() method for dictionaries doesn't exist in Python3. Looks like
items() can be used instead.
http://docs.python.org/3.1/whatsnew/3.0.html#views-and-iterators-instead-of-lists
Also, the parenthesis-less print statements won't work, but they look mostly
temporary.
Running with latest trunk, I noticed two of the tests fail
(lua_testsuite.LuaTestsuiteInGame and lua_persistence.LuaPersistence), and I
think only the former failed yesterday...
--
https://code.launchpad.net/~widelands-dev/widelands/regression_testing/+merge/178000
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/regression_testing into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
