pynfs: cleanup and turn off some tests by default
Subsystems (2)
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
From: "J. Bruce Fields" <[email protected]>
The printk doesn't seem especially useful, and it's just clutter if you want to use output of the scripts somehow.
--- nfs4.0/nfs4lib.py | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
nfs4.0/nfs4lib.py +0 -1
@@ -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):
Signature · 1 line
1.7.5.4
PATCH 2/5 4.0 tests: remove 1-second wait on client creation
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
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.)
--- nfs4.0/nfs4lib.py | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
nfs4.0/nfs4lib.py +0 -1
@@ -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:
Signature · 1 line
1.7.5.4
PATCH 3/5 4.1 server tests: don't run pnfs tests by default
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
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.
--- nfs4.1/server41tests/st_current_stateid.py | 2 +- nfs4.1/server41tests/st_getdevicelist.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-)
nfs4.1/server41tests/st_current_stateid.py +1 -1
@@ -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),
nfs4.1/server41tests/st_getdevicelist.py +8 -8
@@ -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
Signature · 1 line
1.7.5.4
PATCH 4/5 4.1 server tests: don't run utf8 tests by default
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
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.
--- 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(-)
nfs4.0/servertests/st_compound.py +1 -1
@@ -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
nfs4.0/servertests/st_link.py +1 -1
@@ -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 """
nfs4.0/servertests/st_lookup.py +1 -1
@@ -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 """
nfs4.0/servertests/st_open.py +1 -1
@@ -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 """
nfs4.0/servertests/st_remove.py +1 -1
@@ -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 """
nfs4.0/servertests/st_rename.py +2 -2
@@ -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 """
nfs4.0/servertests/st_secinfo.py +1 -1
@@ -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 """
nfs4.0/servertests/st_setattr.py +1 -1
@@ -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 """
nfs4.1/server41tests/st_compound.py +1 -1
@@ -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
nfs4.1/server41tests/st_rename.py +2 -2
@@ -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)
Signature · 1 line
1.7.5.4
PATCH 5/5 4.0 server tests: remove RD12 (incorrect test)
In reply to J. Bruce FieldsFrom: "J. Bruce Fields" <[email protected]>
Quoted 1 line
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).
--- nfs4.0/servertests/st_read.py | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-)
nfs4.0/servertests/st_read.py +0 -14
@@ -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])
Signature · 1 line
1.7.5.4
Quoted 7 lines · 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.
Quoted 5 lines
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"
Quoted 2 lines
--b.
Thanks
Boaz
Quoted 8 lines · Boaz Harrosh
Quoted 7 lines · 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 15 lines · J. Bruce Fields
Quoted 14 lines · Boaz Harrosh
Quoted 12 lines · 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.
Quoted 9 lines
Quoted 2 lines
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
Quoted 4 lines
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 ;-) )
Quoted 1 line
--b.
Thanks
Boaz