Please use knownfail rather than skip

[David Disseldorp]

Hi Andrew,

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
suitable.
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.

True.

> 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
> it.

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.

Cheers, David