Anyone else care to join in? I need another brain! I'm tempted to cross-post to perlunit-devel... maybe you could reply about the patch/refactor to devel, and discuss the actual use of decoupled test cases on user?
On Tue, Oct 26, 2004 at 02:17:28PM +0200, Jean-Louis Leroy wrote: > I've found out a trivial bug in Test::Unit, You're right that @TESTS doesn't behave as documented. The rest of the problem is more complex, but it makes sense to: correct the docs, ensure the users can do what they _need_ to do, hopefully give them a little nudge to help differentiate between wanting ordered tests and actually needing them. > which btw solves this problem: > http://sourceforge.net/mailarchive/forum.php?thread_id=3232369&forum_id=2441 Thanks for digging out the reference. 8-) > Test::Unit::TestCase::list_tests does grab the testsub names from > @TESTS as documented, but then sticks them in a hash, probably to > remove duplicates: The comments here are relevant, # Returns a list of the tests run by this class and its superclasses. # DO NOT OVERRIDE THIS UNLESS YOU KNOW WHAT YOU ARE DOING! > sub list_tests { > my $class = ref($_[0]) || $_[0]; > # ... > my %tests = map {$_ => ''} @tests; > return keys %tests; > } I don't know the exact reason for the shouty warning. I think it's justified but needs another 300 words of explanation. Hmm, lots of things going on in this method. Looking at the code and pondering what's needed: In the general case it makes sense to shuffle the order of the tests methods: AIUI the design intention is that tests should be independent of each other. This includes side effects, previous return values and so on. Illustrations below. I thought we had discussed the explicit use of rand() in shuffling the tests, but can't find mention in Google. rand() could produce unnerving intermittent failures if there were linkage between tests, but is better than tests that fail or not depending on what other test methods are present, the names of the tests and the version of Perl. The introspection (get_matching_methods calls) can only produce duplicates when a test has been overridden from its superclasses. It makes sense to filter these out: the overridden methods can't be called by name and when overriding a method one expects it to disappear from the bottom layer. I can't think why one might override a test, but that's beside the point. So class methods and superclass methods are shuffled together and de-duped and this is fine so far. When @TESTS is set, the superclasses' tests are _appended_ to the ones listed in @TESTS, then shuffled. Failing to include them now is likely to confuse people so we'd better not change that. But for the shuffle, you could put the superclass tests first by including their names in @TESTS at the front. Having a method to get them might be convenient. Superclass test methods are added in an order depending on how they're shuffled in their class and the inheritance order; unless you also fixed the sort for these classes by whatever means. Test names still need de-duping and your %seen will do this. Or do they? Maybe you put the name of one test in @TESTS several times, because you wanted it run several times? This isn't how it was intended to work, but TMTOWTDI says maybe it should be possible. Most people would still want supermethods de-duped. So we have test ordering (by hash, random, as specified, sorted), and the relative ordering of tests vs. inherited tests. The only order that can't easily be reconstructed is "as specified", so it makes sense to preserve this where possible. We don't want repeat runs of overriding tests, but may want repeat runs explicitly in @TESTS. We want to keep the current default of hash ordered tests. Switching to random ordering for the default needs wider discussion. One solution would be a Test::Unit::OrderPreservedTestCase which contains the current list_tests method, minus the %tests but plus a de-dup filter on the inherited tests. Then the Test::Unit::TestCase inherits from the thing with the tedious name and does the %tests thing. This pushes the complications out into the already complicated class structure. Another solution would involve methods such as list_local_tests_ordered # returns @TESTS or introspects class list_super_tests_ordered # recurses on @ISA classes list_tests_ordered ..... # combines (@local, @super), de-duping # @super but not @local list_tests # get ordered list, shuffle by hash Then we add good comments about why it's done like this and how to get the most from it. Folks determined to sort test names can override list_tests with a pass-through method. Folks wanting proper randomisation can override list_tests to shuffle properly. Default behaviour stays the same, except @TESTS works as documented. Folks wishing to skip even numbered test methods after the shuffle can do this, provided the thread on the other CPU shuffles with the same random seed. Any more? > I experimented a fix in a subclass of T::U::TestCase: > > sub list_tests { > my $class = ref($_[0]) || $_[0]; > # ... > my %seen; > return grep { !$seen{$_}++ } @tests; > } I'm glad this will fix your problem. I think we need something more flexible for the default behaviour, and there may be better ways to fix your problem depending on exactly what it is. > I see that the latest release of the module dates back to 2002, Yes. There are a couple of changes since then in CVS, but nothing exciting. > and the latest message on this ML is one year old. There's more recent activity on the perlunit-devel list, for small values of 'activity'. > Is the module still alive? Who's in charge? Many developers have been quiet for a while, I can't speak for them. I believe Piers has the CPAN upload password. I am using perlunit again at (new) work and have some interest in at least the bugfixes. Of which this is one. > If I take the time to make a proper patch, will it be incorporated > and will a new version be released? Yes and no. We will happily accept patches, and future releases will happen at some point. Exactly who is "we" and when things happen mostly depends on our round-tuit schedulers. More importantly for your patch, I think the fix should either be to the documentation, or a wider change to the code. Will you stay for a chat before leaving your patch? 8-) The JUnit guys discuss test execution order starting at http://groups.yahoo.com/group/junit/message/4063 and ending with a removal of the OP's need to fix order at http://groups.yahoo.com/group/junit/message/4068 The Ant (build tool) guys have a go too, http://www.mail-archive.com/[EMAIL PROTECTED]/msg08101.html but that's broken. Ask Google for "run junit tests in a certain order" while it's still in the cache, or try http://web.archive.org . There's more about the design at http://www.martinfowler.com/bliki/JunitNewInstance.html but not much discussion of the ordering. A more concrete illustration involving databases, typed off the top of my head so please excuse typos, package SortmeTest; use strict; use base 'Test::Unit::TestCase'; our @TESTS = qw( test_insert test_select ); # assume this works as documented # else do # sub list_tests { return sort $_[0]->SUPER::list_tests } # just like the code tells you not to my $dbh; sub set_up { my $self = shift; $self->SUPER::set_up; # always a good idea $dbh ||= new DBI(gubbins); # ensure we have a database handle } sub test_insert { my $self = shift; my $expect = 1; # daft for a simple example, but a good pattern my $actual = $dbh->do("insert into tble (a, b) values (3, 42)"); $self->assert_equals( $expect, $actual ); } sub test_select { my $self = shift; my $rows = $dbh->selectall_arrayref("select b from tble where a = 3"); $self->assert_equals(1, scalar @$rows); # check num. rows $self->assert_equals(42, $rows->[0]->[0]); # check the b value } END { if ($dbh) { $dbh->rollback; # so the insert doesn't fail tomorrow undef $dbh; } } There are many places where you have to force the design to do something odd: - global database handle Can't use an object instance variable because different tests have different objects. May prefer not to have two database connections because it will be slower. Can't use the transaction to do insert/ select/rollback, except on one handle. - sorting the method execution order Arises from the dependence of test_select on the preceding test_insert. If you maintain the list of test methods independently of the method definitions, they can get out of step. This will bite you. - the magic number 42 appears in two places Fine for a small constant, but what about a 70 character text constant? Use another global? Risk a typo? - use of END instead of tear_down If you run in the Tk test runner, this could be hours later since you'll be examining the test output before quitting perl. Suppose you're on Oracle and you have a unique index. The uncommitted record will cause a subsequent run of the test suite to hang until you close the window on the first one. (By default? I don't know about disabling the behaviour.) Very confusing. Fixing this one by having a teardown which checks $self->name before deciding whether to rollback... well this way lies further madness. OK, so I have set up my straw man. He won't mind being "mended" with a small diff: package ImprovedTest; use strict; use base 'Test::Unit::TestCase'; sub set_up { my $self = shift; $self->SUPER::set_up; # always a good idea $self->{dbh} = new DBI(gubbins); } sub dbh { $_[0]->{dbh} } # if you like accessors sub test_fortytwo { my $self = shift; my $num = 42; $self->part_insert($num); $self->part_select($num); } sub part_insert { my ($self, $val) = @_; my $expect = 1; # daft for a simple example, but a good pattern my $actual = $self->dbh->do("insert into tble (a, b) values (3, $val)"); $self->assert_equals( $expect, $actual ); } sub part_select { my ($self, $val) = @_; my $rows = $self->dbh->selectall_arrayref("select b from tble where a = 3"); $self->assert_equals(1, scalar @$rows); # check num. rows $self->assert_equals($val, $rows->[0]->[0]); # check the b value } sub tear_down { my $self = shift; $self->SUPER::tear_down; my $dbh = $self->dbh; if ($dbh) { $dbh->rollback; # so the insert doesn't fail tomorrow } } Does this example make things any clearer? - one dbh per test object = one dbh per insert/select/rollback We have isolation. - test suite runs your tests independently Currently in series but no particular order, but if you're on a multi-processor machine then maybe you prefer to run them in parallel? Or with a cluster, on separate machines? These features are just a twinkle in the eye, now. One day somebody with a three hour test suite will implement them because it would save time to do so... and might be easier than optimising the tests. - change your magic constants with ease I can add a new test like this, sub test_elephant { my $self = shift; my $string = "elephant"; $self->part_insert($string); $self->part_select($string); } when I decide that the tble shall also accept strings. - rollback happens ASAP, whether tests pass/fail/die In a general way, for any other test methods we declare. This starts to look like a base class My::DatabaseTestCase if you have many groups of tests to write. - doesn't even try the select if the insert fails The failure exception or die takes control off to the framework and then to tear_down. No point trying the select, you expect it to fail. If you want to ensure that the select _does_ fail when run without the insert, this is a separate test. sub test_selectfail { my $self = shift; $self->assert_raises('Test::Unit::Failure', sub { $self->part_select("Elvis") }); } It's probably a useful test, too: a fresh dbh, select, failure, catch failure and be happy with it. I hope this gives you some ideas. Sorry if it's too simple for you, but I haven't seen the tests you already have. Matthew #8-) ------------------------------------------------------- This SF.Net email is sponsored by: Sybase ASE Linux Express Edition - download now for FREE LinuxWorld Reader's Choice Award Winner for best database on Linux. http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click _______________________________________________ Perlunit-users mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/perlunit-users
