Reviewing merge request #2318: Added -random option to qtestlib
ref: http://qt.gitorious.org/qt/qt/merge_requests/2316
Added -random option to tests, making the test cases within a test execute in arbitrary order. Very useful for avoiding test cases being dependent on the running order.
Most other unit test frameworks have such an option, but found no way of implementing it on top of the Qt test library, so here's a merge request
New version, removed QLists, rebased on master.
- Requested by:
Magne Zachrisen - Status: Rejected
- Source repository: magnezs-qt:master-qtestpatch-2
- Target repository: qt:master
- Created at: February 24th, 2010 12:57
Commits that would be merged:
- fd3f9dd
- 4ac0cce
Added -random option to tests, making the test cases within a test execute in
fd3f9dd-4ac0cceComments
Thanks, looks better now. I need a second opinion, someone from QA preferrably.
It’s a cool feature, I vote for it.
Concept is good.
Code is pretty good but I think you should initialize testFuncs and testFuncCount to 0 (testFuncs really should have been initialized before your change, but having a static deleter of testFuncs makes it more important I guess).
It’s missing an autotest, though. Can you try to add an autotest under tests/auto/selftests for this? Since we’re testing “random” behaviour, it’ll be a little trickier than the existing selftests – maybe you could run a testcase twice, and verify that all the testfunctions were run and the order was different both times.
Yeah I guess I could explicitly initialize them to zero just to show that I didn’t forget.
I was thinking of adding another parameter “-seed” so that it is possible to reproduce the sequence if a certain sequence fails. This’ll make it possible to write deterministic selftests for the -random feature also.
Done. Added -seed option to make it possible to make reproducable random unit test results, also making it possible to test the testlib code.
Fixed the uninitialized statics and added auto-tests for the new options
See new merge request:
Closed this merge request – now follow 2362.
Add a new comment:
Login or create an account to post a comment