pynfs: cleanup and turn off some tests by default

pynfs: cleanup and turn off some tests by default

[J. Bruce Fields]

I've committed the following to my pynfs repo.

Fix a couple minor problems found while working on a bug, then turn off
utf8 and pnfs tests by default (both can still be requested by adding
"utf8" or "pnfs", respectively, on the commandline), and remove one
incorrect test.

I'd like to get to the point where the pynfs tests all pass against a
reasonable server.  That means fixing the server where necessary, of
course, but also not complaining about unimplemented optional features
or about stuff we don't plan to fix.

--b.

[PATCH 1/5] 4.0 tests: kill "Starting Call Back server stub" printk

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

The printk doesn't seem especially useful, and it's just clutter if
you want to use output of the scripts somehow.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 nfs4.0/nfs4lib.py |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/nfs4.0/nfs4lib.py b/nfs4.0/nfs4lib.py
index 0a38ebc..c6d6781 100644
--- a/nfs4.0/nfs4lib.py
+++ b/nfs4.0/nfs4lib.py
@@ -191,7 +191,6 @@ class CBServer(rpc.RPCServer):
             raise "Bad name %s" % name
 
     def run(self):
-        print "Starting Call Back server stub on port %i" % self.port
         rpc.RPCServer.run(self)
 
     def handle_0(self, data, cred):
-- 
1.7.5.4

[PATCH 2/5] 4.0 tests: remove 1-second wait on client creation

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

I can't figure out why this wait is necessary, and it slows things down.
(Noticed while trying to write a race-condition reproducer requiring
more precise timing.)

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 nfs4.0/nfs4lib.py |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/nfs4.0/nfs4lib.py b/nfs4.0/nfs4lib.py
index c6d6781..7185057 100644
--- a/nfs4.0/nfs4lib.py
+++ b/nfs4.0/nfs4lib.py
@@ -293,7 +293,6 @@ class NFS4Client(rpc.RPCClient, nfs4_ops.NFS4Operations):
         self.cb_control = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         while 1:
             try:
-                time.sleep(1)
                 self.cb_control.connect(('127.0.0.1', self.cb_server.port))
                 break
             except socket.error:
-- 
1.7.5.4

[PATCH 3/5] 4.1 server tests: don't run pnfs tests by default

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

I'd rather the pynfs tests have clean results by default against a
correct 4.1 server.  And it's correct for such a server not to implement
optional features.  So leave pnfs tests off by default; you can still
get them by asking for "pnfs" tests.

Also, flags for individual pnfs operations seemed mild overkill for now,
given how few tests we have.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 nfs4.1/server41tests/st_current_stateid.py |    2 +-
 nfs4.1/server41tests/st_getdevicelist.py   |   16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/nfs4.1/server41tests/st_current_stateid.py b/nfs4.1/server41tests/st_current_stateid.py
index aeaa8b1..7804900 100644
--- a/nfs4.1/server41tests/st_current_stateid.py
+++ b/nfs4.1/server41tests/st_current_stateid.py
@@ -119,7 +119,7 @@ def testOpenLayoutGet(t, env):
     """test current state id processing by having OPEN and LAYOUTGET
        in a single compound
 
-    FLAGS: currentstateid all
+    FLAGS: currentstateid pnfs
     CODE: CSID7
     """
     sess = env.c1.new_client_session(env.testname(t),
diff --git a/nfs4.1/server41tests/st_getdevicelist.py b/nfs4.1/server41tests/st_getdevicelist.py
index 8d53633..7a10c6c 100644
--- a/nfs4.1/server41tests/st_getdevicelist.py
+++ b/nfs4.1/server41tests/st_getdevicelist.py
@@ -10,7 +10,7 @@ from nfs4lib import state00
 def testGetDevList(t, env):
     """Check devlist
 
-    FLAGS: getdevicelist all
+    FLAGS: pnfs
     CODE: GETDLIST1
     """
     # Make sure E_ID returns MDS capabilities
@@ -36,7 +36,7 @@ def testGetDevList(t, env):
 def testGetDevInfo(t, env):
     """Check devlist
 
-    FLAGS: getdevicelist all
+    FLAGS: pnfs
     DEPEND: GETDLIST1
     CODE: GETDINFO1
     """
@@ -73,7 +73,7 @@ def testGetDevInfo(t, env):
 ## def xxxtestLayout(t, env):
 ##     """Verify layout handling
 
-##     FLAGS: layout all
+##     FLAGS: pnfs
 ##     CODE: GETLAYOUT1
 ##     """
 ##     # Make sure E_ID returns MDS capabilities
@@ -105,7 +105,7 @@ def testGetDevInfo(t, env):
 def testGetLayout(t, env):
     """Verify layout handling
 
-    FLAGS: getlayout all
+    FLAGS: pnfs
     CODE: GETLAYOUT1
     """
     # Make sure E_ID returns MDS capabilities
@@ -187,7 +187,7 @@ def testLayoutReturnFile(t, env):
     """
     Return a file's layout
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET1
     """
@@ -220,7 +220,7 @@ def testLayoutReturnFsid(t, env):
     """
     Return all of a filesystem's layouts
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET2
     """
@@ -252,7 +252,7 @@ def testLayoutReturnAll(t, env):
     """
     Return all of a client's layouts
     
-    FLAGS: layoutreturn all
+    FLAGS: pnfs
     DEPEND: GETLAYOUT1
     CODE: LAYOUTRET3
     """
@@ -283,7 +283,7 @@ def testLayoutCommit(t, env):
     """
     Do some commits
 
-    FLAGS: layoutcommit all
+    FLAGS: pnfs
     CODE: LAYOUTCOMMIT1
     """
     # Make sure E_ID returns MDS capabilities
-- 
1.7.5.4

[PATCH 4/5] 4.1 server tests: don't run utf8 tests by default

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

The server doesn't pass the utf8 tests and probably never will, and I'm
tired of people reporting those failures to me.

It's unclear to me whether those tests are even completely correct given
the latest internationalization language in the spec.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 nfs4.0/servertests/st_compound.py   |    2 +-
 nfs4.0/servertests/st_link.py       |    2 +-
 nfs4.0/servertests/st_lookup.py     |    2 +-
 nfs4.0/servertests/st_open.py       |    2 +-
 nfs4.0/servertests/st_remove.py     |    2 +-
 nfs4.0/servertests/st_rename.py     |    4 ++--
 nfs4.0/servertests/st_secinfo.py    |    2 +-
 nfs4.0/servertests/st_setattr.py    |    2 +-
 nfs4.1/server41tests/st_compound.py |    2 +-
 nfs4.1/server41tests/st_rename.py   |    4 ++--
 10 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/nfs4.0/servertests/st_compound.py b/nfs4.0/servertests/st_compound.py
index 98be3a8..d2af73b 100644
--- a/nfs4.0/servertests/st_compound.py
+++ b/nfs4.0/servertests/st_compound.py
@@ -31,7 +31,7 @@ def testGoodTag(t, env):
 def testBadTags(t, env):
     """COMPOUND with invalid utf8 tags
 
-    FLAGS: compound utf8 all
+    FLAGS: compound utf8
     CODE: COMP3
     """
     c = env.c1
diff --git a/nfs4.0/servertests/st_link.py b/nfs4.0/servertests/st_link.py
index 10d7197..e20ea77 100644
--- a/nfs4.0/servertests/st_link.py
+++ b/nfs4.0/servertests/st_link.py
@@ -212,7 +212,7 @@ def testLongName(t, env):
 def testInvalidUtf8(t, env):
     """LINK with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: link utf8 all
+    FLAGS: link utf8
     DEPEND: LINKS LOOKFILE MKDIR
     CODE: LINK8
     """
diff --git a/nfs4.0/servertests/st_lookup.py b/nfs4.0/servertests/st_lookup.py
index 40c5983..456fed2 100644
--- a/nfs4.0/servertests/st_lookup.py
+++ b/nfs4.0/servertests/st_lookup.py
@@ -221,7 +221,7 @@ def testNonAccessable(t, env):
 def testInvalidUtf8(t, env):
     """LOOKUP with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: lookup utf8 all
+    FLAGS: lookup utf8
     DEPEND:
     CODE: LOOK7
     """
diff --git a/nfs4.0/servertests/st_open.py b/nfs4.0/servertests/st_open.py
index 8176ad5..992e5e3 100644
--- a/nfs4.0/servertests/st_open.py
+++ b/nfs4.0/servertests/st_open.py
@@ -247,7 +247,7 @@ def testNotDir(t, env):
 def testInvalidUtf8(t, env):
     """OPEN with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: open utf8 all
+    FLAGS: open utf8
     DEPEND: MKDIR
     CODE: OPEN13
     """
diff --git a/nfs4.0/servertests/st_remove.py b/nfs4.0/servertests/st_remove.py
index 76a7037..d475185 100644
--- a/nfs4.0/servertests/st_remove.py
+++ b/nfs4.0/servertests/st_remove.py
@@ -195,7 +195,7 @@ def testZeroLengthTarget(t, env):
 def testNonUTF8(t, env):
     """REMOVE with non-UTF8 components should return NFS4ERR_INVAL
 
-    FLAGS: remove utf8 all
+    FLAGS: remove utf8
     DEPEND: MKDIR
     CODE: RM5
     """
diff --git a/nfs4.0/servertests/st_rename.py b/nfs4.0/servertests/st_rename.py
index fe6339b..16e4ce5 100644
--- a/nfs4.0/servertests/st_rename.py
+++ b/nfs4.0/servertests/st_rename.py
@@ -311,7 +311,7 @@ def testZeroLengthNewname(t, env):
 def testBadutf8Oldname(t, env):
     """RENAME with non-UTF8 oldname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     DEPEND: MKDIR
     CODE: RNM8
     """
@@ -327,7 +327,7 @@ def testBadutf8Oldname(t, env):
 def testBadutf8Newname(t, env):
     """RENAME with non-UTF8 newname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     DEPEND: MKDIR
     CODE: RNM9
     """
diff --git a/nfs4.0/servertests/st_secinfo.py b/nfs4.0/servertests/st_secinfo.py
index 793fdfd..83f713b 100644
--- a/nfs4.0/servertests/st_secinfo.py
+++ b/nfs4.0/servertests/st_secinfo.py
@@ -80,7 +80,7 @@ def testZeroLenName(t, env):
 def testInvalidUtf8(t, env):
     """SECINFO with bad UTF-8 name strings should return NFS4ERR_INVAL
 
-    FLAGS: secinfo utf8 all
+    FLAGS: secinfo utf8
     DEPEND: SEC1
     CODE: SEC6
     """
diff --git a/nfs4.0/servertests/st_setattr.py b/nfs4.0/servertests/st_setattr.py
index 686ce87..936778f 100644
--- a/nfs4.0/servertests/st_setattr.py
+++ b/nfs4.0/servertests/st_setattr.py
@@ -412,7 +412,7 @@ def testNonUTF8(t, env):
     OWNER and GROUP are subject to too many restrictions to use.
     Similarly for ACL.
 
-    FLAGS: setattr utf8 all
+    FLAGS: setattr utf8
     DEPEND: MKFILE
     CODE: SATT9
     """
diff --git a/nfs4.1/server41tests/st_compound.py b/nfs4.1/server41tests/st_compound.py
index 4dedca8..d38f621 100644
--- a/nfs4.1/server41tests/st_compound.py
+++ b/nfs4.1/server41tests/st_compound.py
@@ -39,7 +39,7 @@ def testGoodTag(t, env):
 def testBadTags(t, env):
     """COMPOUND with invalid utf8 tags
 
-    FLAGS: compound utf8 all
+    FLAGS: compound utf8
     CODE: COMP3
     """
     c = env.c1
diff --git a/nfs4.1/server41tests/st_rename.py b/nfs4.1/server41tests/st_rename.py
index bddcb35..ed5dbb5 100644
--- a/nfs4.1/server41tests/st_rename.py
+++ b/nfs4.1/server41tests/st_rename.py
@@ -314,7 +314,7 @@ def testZeroLengthNewname(t, env):
 def testBadutf8Oldname(t, env):
     """RENAME with non-UTF8 oldname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     CODE: RNM8
     """
     name = env.testname(t)
@@ -330,7 +330,7 @@ def testBadutf8Oldname(t, env):
 def testBadutf8Newname(t, env):
     """RENAME with non-UTF8 newname should return NFS4ERR_INVAL
 
-    FLAGS: rename utf8 all
+    FLAGS: rename utf8
     CODE: RNM9
     """
     name = env.testname(t)
-- 
1.7.5.4

[PATCH 5/5] 4.0 server tests: remove RD12 (incorrect test)

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

>From 3530 section 8.1.4:

	In the case of WRITE-type operations (i.e., WRITEs and SETATTRs
	which set size), the server must verify that the access mode
	allows writing and return an NFS4ERR_OPENMODE error if it does
	not.  In the case, of READ, the server may perform the
	corresponding check on the access mode, or it may choose to
	allow READ on opens for WRITE only, to accommodate clients whose
	write implementation may unavoidably do reads (e.g., due to
	buffer cache constraints).

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 nfs4.0/servertests/st_read.py |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/nfs4.0/servertests/st_read.py b/nfs4.0/servertests/st_read.py
index 639b436..cce9a03 100644
--- a/nfs4.0/servertests/st_read.py
+++ b/nfs4.0/servertests/st_read.py
@@ -219,17 +219,3 @@ def testOldStateid(t, env):
     fh, stateid = c.confirm(t.code, res)
     res = c.read_file(fh, stateid=oldstateid)
     check(res, NFS4ERR_OLD_STATEID, "READ with old stateid")
-
-def testOpenMode(t, env):
-    """READ with file opened in WRITE mode should return NFS4_OK or NFS4ERR_OPENMODE
-
-    FLAGS: read all
-    DEPEND: MKFILE
-    CODE: RD12
-    """
-    c = env.c1
-    c.init_connection()
-    fh, stateid = c.create_confirm(t.code, access=OPEN4_SHARE_ACCESS_WRITE)
-    res = c.read_file(fh, stateid=stateid)
-    check(res, NFS4ERR_OPENMODE, "READ with file opened in WRITE mode",
-          [NFS4_OK])
-- 
1.7.5.4

Re: pynfs: cleanup and turn off some tests by default

[Boaz Harrosh]

On 02/15/2012 02:45 PM, J. Bruce Fields wrote:
> I've committed the following to my pynfs repo.
> 
> Fix a couple minor problems found while working on a bug, then turn off
> utf8 and pnfs tests by default (both can still be requested by adding
> "utf8" or "pnfs", respectively, on the commandline), and remove one
> incorrect test.
> 
> I'd like to get to the point where the pynfs tests all pass against a
> reasonable server.  That means fixing the server where necessary, of
> course, but also not complaining about unimplemented optional features
> or about stuff we don't plan to fix.
> 

Can't we "auto detect" the pnfs option. Request a pnfs access if failed
proceed to the next set of tests, if successful continue to test "pnfs"

> --b.
> 

Thanks
Boaz

Re: pynfs: cleanup and turn off some tests by default

[J. Bruce Fields]

On Wed, Feb 15, 2012 at 04:28:01PM -0800, Boaz Harrosh wrote:
> On 02/15/2012 02:45 PM, J. Bruce Fields wrote:
> > I've committed the following to my pynfs repo.
> > 
> > Fix a couple minor problems found while working on a bug, then turn off
> > utf8 and pnfs tests by default (both can still be requested by adding
> > "utf8" or "pnfs", respectively, on the commandline), and remove one
> > incorrect test.
> > 
> > I'd like to get to the point where the pynfs tests all pass against a
> > reasonable server.  That means fixing the server where necessary, of
> > course, but also not complaining about unimplemented optional features
> > or about stuff we don't plan to fix.
> > 
> 
> Can't we "auto detect" the pnfs option. Request a pnfs access if failed
> proceed to the next set of tests, if successful continue to test "pnfs"

Good idea!  I think you'd still want "pnfs" and "nopnfs" options: "pnfs"
for "I expect this server to support pnfs, and if it doesn't report that
as a bug!", and "nopnfs" for: "this server claims to support  pnfs but
please just don't even bother telling me how buggy it is, I already
know".

But just turning it off by default was a minute's worth of
search-and-replace for me, and I don't have the time for more right now.
So that's a project looking for a volunteer.

--b.

Re: pynfs: cleanup and turn off some tests by default

[Boaz Harrosh]

On 02/15/2012 05:28 PM, J. Bruce Fields wrote:
> On Wed, Feb 15, 2012 at 04:28:01PM -0800, Boaz Harrosh wrote:
>> On 02/15/2012 02:45 PM, J. Bruce Fields wrote:
>>> I've committed the following to my pynfs repo.
>>>
>>> Fix a couple minor problems found while working on a bug, then turn off
>>> utf8 and pnfs tests by default (both can still be requested by adding
>>> "utf8" or "pnfs", respectively, on the commandline), and remove one
>>> incorrect test.
>>>
>>> I'd like to get to the point where the pynfs tests all pass against a
>>> reasonable server.  That means fixing the server where necessary, of
>>> course, but also not complaining about unimplemented optional features
>>> or about stuff we don't plan to fix.
>>>
>>
>> Can't we "auto detect" the pnfs option. Request a pnfs access if failed
>> proceed to the next set of tests, if successful continue to test "pnfs"
> 
> Good idea!  I think you'd still want "pnfs" and "nopnfs" options: "pnfs"
> for "I expect this server to support pnfs, and if it doesn't report that
> as a bug!", and "nopnfs" for: "this server claims to support  pnfs but
> please just don't even bother telling me how buggy it is, I already
> know".
> 

Sounds good

> But just turning it off by default was a minute's worth of
> search-and-replace for me, and I don't have the time for more right now.
> So that's a project looking for a volunteer.
> 

NP, thanks for the fixes so far. We'll probably add pynfs testing to our
Ganesha testing rig. When we do we'll look into it. That is if no one
bits us to it, (Hopefully ;-) )

> --b.

Thanks
Boaz