Please use knownfail rather than skip
ddiss at suse.de
Fri Feb 1 04:19:56 MST 2013
On Fri, 01 Feb 2013 07:28:06 +1100
Andrew Bartlett <abartlet at samba.org> wrote:
> On Thu, 2013-01-31 at 19:40 +0100, Andreas Schneider wrote:
> > commit cf27c2fbb6e7422cb962f4c63a53515321c65a70
> > Author: David Disseldorp <ddiss at samba.org>
> > Date: Thu Jan 31 17:48:25 2013 +0100
> > selftest: skip smb2.ioctl tests on ntvfs
> > Rather than filtering via knownfail.
> There may be more behind this than in obvious by this patch, but I need
> to call this out as being a bad pattern to start.
> Unless there is some entirely undesirable side-effect, this patch throws
> away valuable testing, because it removes the test of the testsuite for
> failure (ie, does the test work!), and it removes the documentation of
> the difference between the servers (if someone did add some this to the
> ntvfs server, it would never be tested).
I see your point, but in this case the tests are exercising the same
ntvfs error path ~18 times. Even then, I think a test that explicitly
checks for the correct "unsupported" error response would be more
Also, the documentation for the difference between file servers is only
relocated from selftest/knownfail to selftest/skip.
> It also removes a test that if this unimplemented functionality is
> called, that we don't crash and die.
> There are sometimes good reasons to do things like this - test time for
> example - but just because something isn't supported right now shouldn't
> be enough to skip a test that we can otherwise cheaply run.
> Perhaps I missed the tread discussing the background, but I can't see
The change was made for the (granted small) timing benefit.
> Can you shed some light on the background here, or can we otherwise go
> back to testing this?
If you'd like to reinstate these tests against ntvfs with knownfail,
then please send through a reversion patch and I'll ack it.
More information about the samba-technical