cyrusimap / cassandane

Other
6 stars 11 forks source link

Tests for `cyr_expire`. #28

Closed ajaysusarla closed 7 years ago

ajaysusarla commented 7 years ago

Tests for:

elliefm commented 7 years ago

Haven't tried it out yet, but it looks good so far.

These suites are full of calls to "die" though, which isn't very nice within Cassandane. We basically only want to die if Cassandane is broken; if it's just a test failure, we should call one of the $self->assert...() functions or $self->fail().

Could you add add another commit that fixes all these up? That way people adding more tests in the future have good code to copy instead of bad. :)

The ones like this:

    -f "$basedir/data/user/cassandane/1." || die;

should be rewritten something like this:

    $self->assert(-f "$basedir/data/user/cassandane/1.");

And the ones with the inverted condition like this:

    -f "$basedir/archive/user/cassandane/1." && die;

should be rewritten something like this:

    $self->assert(not -f  "$basedir/archive/user/cassandane/1.")

(or check the -X section of perldoc perlfunc for a suitable inversion of -f and use that instead of not -f ...)

The stuff like this:

     $talk->create($subfolder)
        or die "Cannot create folder $subfolder: $@";

should probably be rewritten like:

     $talk->create($subfolder)
        or $self->fail("Cannot create folder $subfolder: $@");
ajaysusarla commented 7 years ago

@elliefm That's valuable feedback. I shall update the tests.

ajaysusarla commented 7 years ago

@elliefm made the changes you suggested. To check the inverted condition, I'm doing:

$self->assert(! -f  "$basedir/archive/user/cassandane/1.");

Since that seems to be the way done in a many of the examples in perldoc and stackoverflow.