From: Brian Warner Date: Wed, 15 Jul 2009 06:45:10 +0000 (-0700) Subject: Allow tests to pass with -OO by turning some AssertionErrors (the ones that X-Git-Tag: trac-4000~7 X-Git-Url: https://git.rkrishnan.org/components/%22news.html/reliability?a=commitdiff_plain;h=d8ba8c2eb5d4b1b5de448bc9cebe2c81cde4247d;p=tahoe-lafs%2Ftahoe-lafs.git Allow tests to pass with -OO by turning some AssertionErrors (the ones that we actually exercise during tests) into more specific exceptions, so they don't get optimized away. The best rule to follow is probably this: if an exception is worth testing, then it's part of the API, and AssertionError should never be part of the API. Closes #749. --- diff --git a/src/allmydata/client.py b/src/allmydata/client.py index df35c89a..35f55c09 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -27,7 +27,8 @@ from allmydata.unknown import UnknownNode from allmydata.stats import StatsProvider from allmydata.history import History from allmydata.interfaces import IURI, INewDirectoryURI, IStatsProducer, \ - IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, RIStubClient + IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, RIStubClient, \ + UnhandledCapTypeError KiB=1024 MiB=1024*KiB @@ -429,9 +430,10 @@ class Client(node.Node, pollmixin.PollMixin): node = LiteralFileNode(u, self) # LIT else: node = FileNode(u, self, self.download_cache_dirman) # CHK - else: - assert IMutableFileURI.providedBy(u), u + elif IMutableFileURI.providedBy(u): node = MutableFileNode(self).init_from_uri(u) + else: + raise UnhandledCapTypeError("cap is recognized, but has no Node") self._node_cache[u_s] = node # note: WeakValueDictionary return self._node_cache[u_s] diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 42188e7b..6a9672e9 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -482,6 +482,10 @@ class CannotPackUnknownNodeError(Exception): """UnknownNodes (using filecaps from the future that we don't understand) cannot yet be copied safely, so I refuse to copy them.""" +class UnhandledCapTypeError(Exception): + """I recognize the cap/URI, but I cannot create an IFilesystemNode for + it.""" + class IFilesystemNode(Interface): def get_uri(): """ diff --git a/src/allmydata/mutable/common.py b/src/allmydata/mutable/common.py index 869b27d3..49a04ff2 100644 --- a/src/allmydata/mutable/common.py +++ b/src/allmydata/mutable/common.py @@ -53,7 +53,8 @@ class CorruptShareError(Exception): self.shnum, self.reason) - +class UnknownVersionError(Exception): + """The share we received was of a version we don't recognize.""" class ResponseCache: """I cache share data, to reduce the number of round trips used during diff --git a/src/allmydata/mutable/layout.py b/src/allmydata/mutable/layout.py index a110eb1d..ee89128c 100644 --- a/src/allmydata/mutable/layout.py +++ b/src/allmydata/mutable/layout.py @@ -1,6 +1,6 @@ import struct -from common import NeedMoreDataError +from common import NeedMoreDataError, UnknownVersionError PREFIX = ">BQ32s16s" # each version has a different prefix SIGNED_PREFIX = ">BQ32s16s BBQQ" # this is covered by the signature @@ -34,7 +34,9 @@ def unpack_prefix_and_signature(data): k, N, segsize, datalen, o) = unpack_header(data) - assert version == 0 + if version != 0: + raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version) + if len(data) < o['share_hash_chain']: raise NeedMoreDataError(o['share_hash_chain'], o['enc_privkey'], o['EOF']-o['enc_privkey']) @@ -60,7 +62,9 @@ def unpack_share(data): o['enc_privkey'], o['EOF']) = struct.unpack(HEADER, data[:HEADER_LENGTH]) - assert version == 0 + if version != 0: + raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version) + if len(data) < o['EOF']: raise NeedMoreDataError(o['EOF'], o['enc_privkey'], o['EOF']-o['enc_privkey']) @@ -132,7 +136,8 @@ def pack_checkstring(seqnum, root_hash, IV): def unpack_checkstring(checkstring): cs_len = struct.calcsize(PREFIX) version, seqnum, root_hash, IV = struct.unpack(PREFIX, checkstring[:cs_len]) - assert version == 0 # TODO: just ignore the share + if version != 0: # TODO: just ignore the share + raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version) return (seqnum, root_hash, IV) def pack_prefix(seqnum, root_hash, IV, diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py index a5fc9b89..212cd5da 100644 --- a/src/allmydata/test/test_mutable.py +++ b/src/allmydata/test/test_mutable.py @@ -1030,7 +1030,7 @@ class Roundtrip(unittest.TestCase, testutil.ShouldFailMixin, PublishMixin): # should be noted in the servermap's list of problems. if substring: allproblems = [str(f) for f in servermap.problems] - self.failUnless(substring in "".join(allproblems)) + self.failUnlessIn(substring, "".join(allproblems)) return servermap if should_succeed: d1 = self._fn.download_version(servermap, ver) @@ -1049,9 +1049,9 @@ class Roundtrip(unittest.TestCase, testutil.ShouldFailMixin, PublishMixin): return d def test_corrupt_all_verbyte(self): - # when the version byte is not 0, we hit an assertion error in - # unpack_share(). - d = self._test_corrupt_all(0, "AssertionError") + # when the version byte is not 0, we hit an UnknownVersionError error + # in unpack_share(). + d = self._test_corrupt_all(0, "UnknownVersionError") def _check_servermap(servermap): # and the dump should mention the problems s = StringIO() diff --git a/src/allmydata/test/test_uri.py b/src/allmydata/test/test_uri.py index 7ac63469..0898530d 100644 --- a/src/allmydata/test/test_uri.py +++ b/src/allmydata/test/test_uri.py @@ -186,10 +186,10 @@ class Constraint(unittest.TestCase): def test_constraint(self): good="http://127.0.0.1:3456/uri/URI%3ADIR2%3Agh3l5rbvnv2333mrfvalmjfr4i%3Alz6l7u3z3b7g37s4zkdmfpx5ly4ib4m6thrpbusi6ys62qtc6mma/" uri.NewDirectoryURI.init_from_human_encoding(good) - self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_string, good) + self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_string, good) bad = good + '===' - self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_human_encoding, bad) - self.failUnlessRaises(AssertionError, uri.NewDirectoryURI.init_from_string, bad) + self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_human_encoding, bad) + self.failUnlessRaises(uri.BadURIError, uri.NewDirectoryURI.init_from_string, bad) fileURI = 'URI:CHK:gh3l5rbvnv2333mrfvalmjfr4i:lz6l7u3z3b7g37s4zkdmfpx5ly4ib4m6thrpbusi6ys62qtc6mma:3:10:345834' uri.CHKFileURI.init_from_string(fileURI) diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index 28905ef4..f7d45da0 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -19,7 +19,8 @@ from allmydata.util.assertutil import precondition from allmydata.test.common import FakeDirectoryNode, FakeCHKFileNode, \ FakeMutableFileNode, create_chk_filenode, WebErrorMixin, ShouldFailMixin from allmydata.interfaces import IURI, INewDirectoryURI, \ - IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, IMutableFileNode + IReadonlyNewDirectoryURI, IFileURI, IMutableFileURI, IMutableFileNode, \ + UnhandledCapTypeError from allmydata.mutable import servermap, publish, retrieve import common_util as testutil from allmydata.test.no_network import GridTestMixin @@ -74,8 +75,9 @@ class FakeClient(service.MultiService): return FakeDirectoryNode(self).init_from_uri(u) if IFileURI.providedBy(u): return FakeCHKFileNode(u, self) - assert IMutableFileURI.providedBy(u), u - return FakeMutableFileNode(self).init_from_uri(u) + if IMutableFileURI.providedBy(u): + return FakeMutableFileNode(self).init_from_uri(u) + raise UnhandledCapTypeError("cap '%s' is recognized, but has no Node" % auri) def create_empty_dirnode(self): n = FakeDirectoryNode(self) diff --git a/src/allmydata/uri.py b/src/allmydata/uri.py index 9be92a41..b37ea8ca 100644 --- a/src/allmydata/uri.py +++ b/src/allmydata/uri.py @@ -7,6 +7,9 @@ from allmydata.util import base32, hashutil from allmydata.interfaces import IURI, IDirnodeURI, IFileURI, IImmutableFileURI, \ IVerifierURI, IMutableFileURI, INewDirectoryURI, IReadonlyNewDirectoryURI +class BadURIError(Exception): + pass + # the URI shall be an ascii representation of the file. It shall contain # enough information to retrieve and validate the contents. It shall be # expressed in a limited character set (namely [TODO]). @@ -60,21 +63,22 @@ class CHKFileURI(_BaseURI): self.total_shares = total_shares self.size = size self.storage_index = hashutil.storage_index_hash(self.key) - assert len(self.storage_index) == 16 - self.storage_index = hashutil.storage_index_hash(key) - assert len(self.storage_index) == 16 # sha256 hash truncated to 128 + if not len(self.storage_index) == 16: # sha256 hash truncated to 128 + raise BadURIError("storage index must be 16 bytes long") @classmethod def init_from_human_encoding(cls, uri): mo = cls.HUMAN_RE.search(uri) - assert mo, uri + if not mo: + raise BadURIError("%s doesn't look like a cap" % (uri,)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)), int(mo.group(3)), int(mo.group(4)), int(mo.group(5))) @classmethod def init_from_string(cls, uri): mo = cls.STRING_RE.search(uri) - assert mo, uri + if not mo: + raise BadURIError("%s doesn't look like a cap" % (uri,)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2)), int(mo.group(3)), int(mo.group(4)), int(mo.group(5))) @@ -212,13 +216,15 @@ class WriteableSSKFileURI(_BaseURI): @classmethod def init_from_human_encoding(cls, uri): mo = cls.HUMAN_RE.search(uri) - assert mo, uri + if not mo: + raise BadURIError("'%s' doesn't look like a cap" % (uri,)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2))) @classmethod def init_from_string(cls, uri): mo = cls.STRING_RE.search(uri) - assert mo, (uri, cls) + if not mo: + raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2))) def to_string(self): @@ -260,13 +266,15 @@ class ReadonlySSKFileURI(_BaseURI): @classmethod def init_from_human_encoding(cls, uri): mo = cls.HUMAN_RE.search(uri) - assert mo, uri + if not mo: + raise BadURIError("'%s' doesn't look like a cap" % (uri,)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2))) @classmethod def init_from_string(cls, uri): mo = cls.STRING_RE.search(uri) - assert mo, uri + if not mo: + raise BadURIError("'%s' doesn't look like a cap" % (uri,)) return cls(base32.a2b(mo.group(1)), base32.a2b(mo.group(2))) def to_string(self): @@ -333,7 +341,8 @@ class _NewDirectoryBaseURI(_BaseURI): @classmethod def init_from_string(cls, uri): mo = cls.BASE_STRING_RE.search(uri) - assert mo, (uri, cls) + if not mo: + raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls)) bits = uri[mo.end():] fn = cls.INNER_URI_CLASS.init_from_string( cls.INNER_URI_CLASS.BASE_STRING+bits) @@ -342,7 +351,8 @@ class _NewDirectoryBaseURI(_BaseURI): @classmethod def init_from_human_encoding(cls, uri): mo = cls.BASE_HUMAN_RE.search(uri) - assert mo, (uri, cls) + if not mo: + raise BadURIError("'%s' doesn't look like a %s cap" % (uri, cls)) bits = uri[mo.end():] while bits and bits[-1] == '/': bits = bits[:-1] diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 6fa84b6a..07f55704 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -12,7 +12,7 @@ import allmydata # to display import path from allmydata import get_package_versions_string from allmydata import provisioning from allmydata.util import idlib, log -from allmydata.interfaces import IFileNode +from allmydata.interfaces import IFileNode, UnhandledCapTypeError from allmydata.web import filenode, directory, unlinked, status, operations from allmydata.web import reliability, storage from allmydata.web.common import abbreviate_size, getxmlfile, WebError, \ @@ -79,7 +79,7 @@ class URIHandler(RenderMixin, rend.Page): try: node = self.client.create_node_from_uri(name) return directory.make_handler_for(node, self.client) - except (TypeError, AssertionError): + except (TypeError, UnhandledCapTypeError, AssertionError): raise WebError("'%s' is not a valid file- or directory- cap" % name) @@ -98,7 +98,7 @@ class FileHandler(rend.Page): # 'name' must be a file URI try: node = self.client.create_node_from_uri(name) - except (TypeError, AssertionError): + except (TypeError, UnhandledCapTypeError, AssertionError): raise WebError("'%s' is not a valid file- or directory- cap" % name) if not IFileNode.providedBy(node):