Unit testing - please try/review

Unit testing - please try/review

by Tim Hunt -
Number of replies: 7
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
The attached patch adds the OU's unit testing framework to Moodle HEAD.

Once you have applied the patch, go to admin and near the bottom you will find a link to run the tests. They take a while to run so, once you have clicked the link, expect to stare at a fairly blank screen for a bit.

Please test this if you can. Also, Martin D, for a checkin of this size, it might be good if you could do a quick code review. (Don't worry, you don't need to look at more than the first 1470 lines of the patch. The rest is the simpletest libraries.) (And do finish the 1.6 relese, have a big party and recover first.)

Don't apply this to a production site!

There is no documentation yet, apart from what is in the source. I'll try to add some pages to the wiki soon. Hopefully it should not need much documentation.

I have tried this on PHP5/Postgres8/Linux, and PHP4/MySQL4/Windows. One of tests fails on MySQL, and one fails on WINDOWS at the moment. Currently, there are only tests for a few datalib methods, and for some general code things like the PHP syntax of each file and trailing whitespace. (Most of the test cases we have written to date are for our new OU-specific code.)

Enjoy!
Average of ratings: -
In reply to Tim Hunt

Re: Unit testing - please try/review

by Dennis Daniels -
Hi Tim!
I'm interested in helping!
I just cked the docs: nothing on how to run a patch
http://docs.moodle.org/en/Special:Search?search=patch&go=Go
or unit
http://docs.moodle.org/en/Special:Search?search=unit&go=Go

Could you supply the *nix commands for running your patch? Is the patch in cvs/contrib? Could it get moved there until it becomes part of the moodle package?

Which directory should I put it in?

wget http://moodle.org/file.php/5/moddata/forum/33/219054/unittest_patch.zip

Presumably you have to unzip it first
unzip unittest_patch.zip (in public_html?)

then
#patch \< unittest_patch (I escaped the less than sign)

Is that correct?

Dennis


In reply to Dennis Daniels

Re: Unit testing - please try/review

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
I use the Eclipse IDE, which has a nice apply patch wizard. But installing that just to apply one patch would be overkill.

The unix command line tool to do the same is 'patch'. I have used it once or twice, but I always have to read the man page and scratch my head to work out the right arguments. Googling for 'how to apply a patch' finds some things, for example.

http://www.ethereal.com/docs/edg_html_chunked/ChSrcPatchApply.html

I have not put this in contrib becuase I am hoping to check it into core once I know it is OK.
In reply to Tim Hunt

Re: Unit testing - please try/review

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers
Thanks, Tim. Mostly it's great and can go into head. Just some small points:

1) Shouldn't be on the admin page there smile Doesn't matter as it's probably getting revamped anyway.

2) To be consistent I guess admin/unittest should actually be admin/report/simpletest and be a proper report.

3) The output format is a bit raw, is it possible to pretty that up without editing simpletest itself?

4) These were odd errors to have (files/directories are readable):

Exception: lib/simpletest/testcode.php -> code_test -> test_dnc -> Unexpected PHP error [file_get_contents(): open_basedir restriction in effect. File(/home/moodtest/public_html/..) is not within the allowed path(s): (/home/moodtest/:/usr/lib/php:/usr/local/lib/php:/tmp)] severity [E_WARNING] in [/home/moodtest/public_html/lib/simpletest/testcode.php] line [42]

Exception: lib/simpletest/testcode.php -> code_test -> test_dnc -> Unexpected PHP error [file_get_contents(/home/moodtest/public_html/..): failed to open stream: Operation not permitted] severity [E_WARNING] in [/home/moodtest/public_html/lib/simpletest/testcode.php] line [42]
In reply to Martin Dougiamas

Re: Unit testing - please try/review

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
1) Shouldn't be on the admin page there smile Doesn't matter as it's probably getting revamped anyway.

So I can check this in as is, and rely on someone else to sort it out?

2) To be consistent I guess admin/unittest should actually be admin/report/simpletest and be a proper report.

OK, I can move the files. Is there any documentation about what one has to do to write a proper report? The Wiki was not very forthcoming:

http://docs.moodle.org/en/Special:Search?search=report

3) The output format is a bit raw, is it possible to pretty that up without editing simpletest itself?

Yes, it is easy to change. All the various bits of results output are controlled by the file ex_reporter.php. You just change the methods like paintPass, paintFail, etc. to change how things are displayed.

So I am happy to change the display, but I have already 'prettied it up' (compared with what we were using in-house) smile.

OK, I have tweaked it a bit more. See the screenshot below. This exhausts my inspiration. If you want it any prettier than that, I am going to need some suggestions on how you think it should look.

4) These were odd errors to have (files/directories are readable):

Yes, it certainly should not be straying above dirroot. There must be a bug. I'll fix it.

Attachment results.png
In reply to Tim Hunt

Re: Unit testing - please try/review

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
Having investigated your odd errors, I am still a bit puzzled. When recursing through folders, it should have been ignoring the directories '.' and '..'.

But then I wondered: Is it possible for is_dir('..') to return false? (becuase of either symbolic links or mount points). I don't really understand enough, but I moved the tests around so it tests for '..' and '.' before everything else, which should make it more robust.
In reply to Tim Hunt

Re: Unit testing - please try/review

by Martin Dougiamas -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Plugin developers Picture of Testers

Thanks, Tim!

1) Best if you don't checkin changes to admin/index.php, because as a report plugin it'll automatically come up under Reports anyway.

2) No problem, you just need a little admin/report/simpletest/mod.html file which gets shown on the main reports file, and this links to whatever script you like for the main business (eg admin/report/simpletest/index.php)

3) Cool, looks a lot better already, and good to know that we can tweak that without messing with simpletest itself.

4) I'll test it again once it's in CVS and look further into it if I see the error again.

In reply to Martin Dougiamas

Re: Unit testing - please try/review

by Tim Hunt -
Picture of Core developers Picture of Documentation writers Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers
OK, I should be able to get this checked in tomorrow.