Fri Mar 25 14:35:14 MDT 2011 wilcoxjg@gmail.com * storage: new mocking tests of storage server read and write There are already tests of read and functionality in test_storage.py, but those tests let the code under test use a real filesystem whereas these tests mock all file system calls. Thu Apr 14 16:48:23 MDT 2011 zooko@zooko.com * test_server.py --> test_backends.py: server.py: added testing of get_latencies in StorageServer This patch test both coverage and handling of small samples in the get_latencies method of StorageServer. get_latencies now distinguishes between highly repetitive latencies and small sample sizes. This is of most concern at the big end of the latency distribution, although the ambiguity increases in general as the sample size decreases. Thu Apr 14 17:15:26 MDT 2011 zooko@zooko.com * test_backends: cleaned whitespace, made test_get_latencies it's own function New patches: [storage: new mocking tests of storage server read and write wilcoxjg@gmail.com**20110325203514 Ignore-this: df65c3c4f061dd1516f88662023fdb41 There are already tests of read and functionality in test_storage.py, but those tests let the code under test use a real filesystem whereas these tests mock all file system calls. ] { addfile ./src/allmydata/test/test_server.py hunk ./src/allmydata/test/test_server.py 1 +from twisted.trial import unittest + +from StringIO import StringIO + +from allmydata.test.common_util import ReallyEqualMixin + +import mock + +# This is the code that we're going to be testing. +from allmydata.storage.server import StorageServer + +# The following share file contents was generated with +# storage.immutable.ShareFile from Tahoe-LAFS v1.8.2 +# with share data == 'a'. +share_data = 'a\x00\x00\x00\x00xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy\x00(\xde\x80' +share_file_data = '\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01' + share_data + +sharefname = 'testdir/shares/or/orsxg5dtorxxeylhmvpws3temv4a/0' + +class TestServerConstruction(unittest.TestCase, ReallyEqualMixin): + @mock.patch('__builtin__.open') + def test_create_server(self, mockopen): + """ This tests whether a server instance can be constructed. """ + + def call_open(fname, mode): + if fname == 'testdir/bucket_counter.state': + raise IOError(2, "No such file or directory: 'testdir/bucket_counter.state'") + elif fname == 'testdir/lease_checker.state': + raise IOError(2, "No such file or directory: 'testdir/lease_checker.state'") + elif fname == 'testdir/lease_checker.history': + return StringIO() + mockopen.side_effect = call_open + + # Now begin the test. + s = StorageServer('testdir', 'testnodeidxxxxxxxxxx') + + # You passed! + +class TestServer(unittest.TestCase, ReallyEqualMixin): + @mock.patch('__builtin__.open') + def setUp(self, mockopen): + def call_open(fname, mode): + if fname == 'testdir/bucket_counter.state': + raise IOError(2, "No such file or directory: 'testdir/bucket_counter.state'") + elif fname == 'testdir/lease_checker.state': + raise IOError(2, "No such file or directory: 'testdir/lease_checker.state'") + elif fname == 'testdir/lease_checker.history': + return StringIO() + mockopen.side_effect = call_open + + self.s = StorageServer('testdir', 'testnodeidxxxxxxxxxx') + + + @mock.patch('time.time') + @mock.patch('os.mkdir') + @mock.patch('__builtin__.open') + @mock.patch('os.listdir') + @mock.patch('os.path.isdir') + def test_write_share(self, mockisdir, mocklistdir, mockopen, mockmkdir, mocktime): + """Handle a report of corruption.""" + + def call_listdir(dirname): + self.failUnlessReallyEqual(dirname, 'testdir/shares/or/orsxg5dtorxxeylhmvpws3temv4a') + raise OSError(2, "No such file or directory: 'testdir/shares/or/orsxg5dtorxxeylhmvpws3temv4a'") + + mocklistdir.side_effect = call_listdir + + class MockFile: + def __init__(self): + self.buffer = '' + self.pos = 0 + def write(self, instring): + begin = self.pos + padlen = begin - len(self.buffer) + if padlen > 0: + self.buffer += '\x00' * padlen + end = self.pos + len(instring) + self.buffer = self.buffer[:begin]+instring+self.buffer[end:] + self.pos = end + def close(self): + pass + def seek(self, pos): + self.pos = pos + def read(self, numberbytes): + return self.buffer[self.pos:self.pos+numberbytes] + def tell(self): + return self.pos + + mocktime.return_value = 0 + + sharefile = MockFile() + def call_open(fname, mode): + self.failUnlessReallyEqual(fname, 'testdir/shares/incoming/or/orsxg5dtorxxeylhmvpws3temv4a/0' ) + return sharefile + + mockopen.side_effect = call_open + # Now begin the test. + alreadygot, bs = self.s.remote_allocate_buckets('teststorage_index', 'x'*32, 'y'*32, set((0,)), 1, mock.Mock()) + print bs + bs[0].remote_write(0, 'a') + self.failUnlessReallyEqual(sharefile.buffer, share_file_data) + + + @mock.patch('os.path.exists') + @mock.patch('os.path.getsize') + @mock.patch('__builtin__.open') + @mock.patch('os.listdir') + def test_read_share(self, mocklistdir, mockopen, mockgetsize, mockexists): + """ This tests whether the code correctly finds and reads + shares written out by old (Tahoe-LAFS <= v1.8.2) + servers. There is a similar test in test_download, but that one + is from the perspective of the client and exercises a deeper + stack of code. This one is for exercising just the + StorageServer object. """ + + def call_listdir(dirname): + self.failUnlessReallyEqual(dirname,'testdir/shares/or/orsxg5dtorxxeylhmvpws3temv4a') + return ['0'] + + mocklistdir.side_effect = call_listdir + + def call_open(fname, mode): + self.failUnlessReallyEqual(fname, sharefname) + self.failUnless('r' in mode, mode) + self.failUnless('b' in mode, mode) + + return StringIO(share_file_data) + mockopen.side_effect = call_open + + datalen = len(share_file_data) + def call_getsize(fname): + self.failUnlessReallyEqual(fname, sharefname) + return datalen + mockgetsize.side_effect = call_getsize + + def call_exists(fname): + self.failUnlessReallyEqual(fname, sharefname) + return True + mockexists.side_effect = call_exists + + # Now begin the test. + bs = self.s.remote_get_buckets('teststorage_index') + + self.failUnlessEqual(len(bs), 1) + b = bs[0] + self.failUnlessReallyEqual(b.remote_read(0, datalen), share_data) + # If you try to read past the end you get the as much data as is there. + self.failUnlessReallyEqual(b.remote_read(0, datalen+20), share_data) + # If you start reading past the end of the file you get the empty string. + self.failUnlessReallyEqual(b.remote_read(datalen+1, 3), '') } [test_server.py --> test_backends.py: server.py: added testing of get_latencies in StorageServer zooko@zooko.com**20110414224823 Ignore-this: 3e266de570f725f768d18c131e2c6d8 This patch test both coverage and handling of small samples in the get_latencies method of StorageServer. get_latencies now distinguishes between highly repetitive latencies and small sample sizes. This is of most concern at the big end of the latency distribution, although the ambiguity increases in general as the sample size decreases. ] { move ./src/allmydata/test/test_server.py ./src/allmydata/test/test_backends.py hunk ./src/allmydata/storage/server.py 134 samples = self.latencies[category][:] samples.sort() count = len(samples) + if count < 1000: + output[category] = None + continue + samples.sort() + stats = {} stats["mean"] = sum(samples) / count stats["01_0_percentile"] = samples[int(0.01 * count)] stats["10_0_percentile"] = samples[int(0.1 * count)] hunk ./src/allmydata/test/test_backends.py 21 sharefname = 'testdir/shares/or/orsxg5dtorxxeylhmvpws3temv4a/0' class TestServerConstruction(unittest.TestCase, ReallyEqualMixin): + @mock.patch('time.time') + @mock.patch('os.mkdir') + @mock.patch('__builtin__.open') + @mock.patch('os.listdir') + @mock.patch('os.path.isdir') + def test_create_server_null_backend(self, mockisdir, mocklistdir, mockopen, mockmkdir, mocktime): + """ This tests whether a server instance can be constructed + with a null backend. The server instance fails the test if it + tries to read or write to the file system. """ + + # Now begin the test. + s = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(),stats_provider=mock.Mock()) + + # The null backend should not talk to the os. + self.failIf(mockisdir.called) + self.failIf(mocklistdir.called) + self.failIf(mockopen.called) + self.failIf(mockmkdir.called) + #self.failIf(mocktime.called) + + # The server's representation should not change. + self.failUnlessReallyEqual(s.__repr__(),'') + + # There should be no latencies when the backend is null. *** The "cancel" category is left out to increase coverage. This seems like a dubious decision. Must consult with more knowledgeable persons. + numbersamples = 1001 + for category in ["allocate","write","close","read","get",\ + "writev","readv","add-lease","renew"]:#,"cancel"]: + [s.add_latency(category,x) for x in numbersamples*[0]] + l = s.get_latencies() + + # Now test that get_latencies correctly reports None for small sample-sizes. + s1 = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(),stats_provider=mock.Mock()) + # There should be no latencies when the backend is null. *** The "cancel" category is left out to increase coverage. This seems like a dubious decision. Must consult with more knowledgeable persons. + numbersamples = 10 + for category in ["allocate","write","close","read","get",\ + "writev","readv","add-lease","renew"]:#,"cancel"]: + [s1.add_latency(category,x) for x in numbersamples*[0]] + l1 = s1.get_latencies() + + for key in l1.keys(): + self.failUnlessReallyEqual(l1[key],None) + # You passed! + + @mock.patch('time.time') + @mock.patch('os.mkdir') @mock.patch('__builtin__.open') def test_create_server(self, mockopen): """ This tests whether a server instance can be constructed. """ } [test_backends: cleaned whitespace, made test_get_latencies it's own function zooko@zooko.com**20110414231526 Ignore-this: 483e7d4eb67cc0cbf780d8219156907e ] { hunk ./src/allmydata/test/test_backends.py 32 tries to read or write to the file system. """ # Now begin the test. - s = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(),stats_provider=mock.Mock()) + s = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(), stats_provider=mock.Mock()) # The null backend should not talk to the os. self.failIf(mockisdir.called) hunk ./src/allmydata/test/test_backends.py 40 self.failIf(mockopen.called) self.failIf(mockmkdir.called) #self.failIf(mocktime.called) - + # The server's representation should not change. hunk ./src/allmydata/test/test_backends.py 42 - self.failUnlessReallyEqual(s.__repr__(),'') + self.failUnlessReallyEqual(s.__repr__(), '') + + # You passed! + + @mock.patch('time.time') + @mock.patch('os.mkdir') + @mock.patch('__builtin__.open') + @mock.patch('os.listdir') + @mock.patch('os.path.isdir') + def test_get_latencies(self, mockisdir, mocklistdir, mockopen, mockmkdir, mocktime): + """ There should be no latencies when the backend is null. *** The "cancel" category is left out to increase coverage. This seems like a dubious decision. Must consult with more knowledgeable persons.""" + + # Now begin the test. + s = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(), stats_provider=mock.Mock()) + + # The null backend should not talk to the os. + self.failIf(mockisdir.called) + self.failIf(mocklistdir.called) + self.failIf(mockopen.called) + self.failIf(mockmkdir.called) + #self.failIf(mocktime.called) hunk ./src/allmydata/test/test_backends.py 64 - # There should be no latencies when the backend is null. *** The "cancel" category is left out to increase coverage. This seems like a dubious decision. Must consult with more knowledgeable persons. numbersamples = 1001 hunk ./src/allmydata/test/test_backends.py 65 - for category in ["allocate","write","close","read","get",\ - "writev","readv","add-lease","renew"]:#,"cancel"]: - [s.add_latency(category,x) for x in numbersamples*[0]] + for category in ["allocate", "write", "close", "read", "get", \ + "writev", "readv", "add-lease", "renew"]:#,"cancel"]: + [s.add_latency(category, x) for x in numbersamples*[0]] l = s.get_latencies() # Now test that get_latencies correctly reports None for small sample-sizes. hunk ./src/allmydata/test/test_backends.py 71 - s1 = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(),stats_provider=mock.Mock()) + s1 = StorageServer('testnodeidxxxxxxxxxx', backend=NullBackend(), stats_provider=mock.Mock()) # There should be no latencies when the backend is null. *** The "cancel" category is left out to increase coverage. This seems like a dubious decision. Must consult with more knowledgeable persons. numbersamples = 10 hunk ./src/allmydata/test/test_backends.py 74 - for category in ["allocate","write","close","read","get",\ - "writev","readv","add-lease","renew"]:#,"cancel"]: - [s1.add_latency(category,x) for x in numbersamples*[0]] + for category in ["allocate", "write", "close", "read", "get", \ + "writev", "readv", "add-lease", "renew"]:#, "cancel"]: + [s1.add_latency(category, x) for x in numbersamples*[0]] l1 = s1.get_latencies() for key in l1.keys(): hunk ./src/allmydata/test/test_backends.py 80 - self.failUnlessReallyEqual(l1[key],None) + self.failUnlessReallyEqual(l1[key], None) # You passed! @mock.patch('time.time') hunk ./src/allmydata/test/test_backends.py 167 bs[0].remote_write(0, 'a') self.failUnlessReallyEqual(sharefile.buffer, share_file_data) - @mock.patch('os.path.exists') @mock.patch('os.path.getsize') @mock.patch('__builtin__.open') } Context: [Fix a test failure in test_package_initialization on Python 2.4.x due to exceptions being stringified differently than in later versions of Python. refs #1389 david-sarah@jacaranda.org**20110411190738 Ignore-this: 7847d26bc117c328c679f08a7baee519 ] [tests: add test for including the ImportError message and traceback entry in the summary of errors from importing dependencies. refs #1389 david-sarah@jacaranda.org**20110410155844 Ignore-this: fbecdbeb0d06a0f875fe8d4030aabafa ] [allmydata/__init__.py: preserve the message and last traceback entry (file, line number, function, and source line) of ImportErrors in the package versions string. fixes #1389 david-sarah@jacaranda.org**20110410155705 Ignore-this: 2f87b8b327906cf8bfca9440a0904900 ] [remove unused variable detected by pyflakes zooko@zooko.com**20110407172231 Ignore-this: 7344652d5e0720af822070d91f03daf9 ] [allmydata/__init__.py: Nicer reporting of unparseable version numbers in dependencies. fixes #1388 david-sarah@jacaranda.org**20110401202750 Ignore-this: 9c6bd599259d2405e1caadbb3e0d8c7f ] [update FTP-and-SFTP.rst: the necessary patch is included in Twisted-10.1 Brian Warner **20110325232511 Ignore-this: d5307faa6900f143193bfbe14e0f01a ] [control.py: remove all uses of s.get_serverid() warner@lothar.com**20110227011203 Ignore-this: f80a787953bd7fa3d40e828bde00e855 ] [web: remove some uses of s.get_serverid(), not all warner@lothar.com**20110227011159 Ignore-this: a9347d9cf6436537a47edc6efde9f8be ] [immutable/downloader/fetcher.py: remove all get_serverid() calls warner@lothar.com**20110227011156 Ignore-this: fb5ef018ade1749348b546ec24f7f09a ] [immutable/downloader/fetcher.py: fix diversity bug in server-response handling warner@lothar.com**20110227011153 Ignore-this: bcd62232c9159371ae8a16ff63d22c1b When blocks terminate (either COMPLETE or CORRUPT/DEAD/BADSEGNUM), the _shares_from_server dict was being popped incorrectly (using shnum as the index instead of serverid). I'm still thinking through the consequences of this bug. It was probably benign and really hard to detect. I think it would cause us to incorrectly believe that we're pulling too many shares from a server, and thus prefer a different server rather than asking for a second share from the first server. The diversity code is intended to spread out the number of shares simultaneously being requested from each server, but with this bug, it might be spreading out the total number of shares requested at all, not just simultaneously. (note that SegmentFetcher is scoped to a single segment, so the effect doesn't last very long). ] [immutable/downloader/share.py: reduce get_serverid(), one left, update ext deps warner@lothar.com**20110227011150 Ignore-this: d8d56dd8e7b280792b40105e13664554 test_download.py: create+check MyShare instances better, make sure they share Server objects, now that finder.py cares ] [immutable/downloader/finder.py: reduce use of get_serverid(), one left warner@lothar.com**20110227011146 Ignore-this: 5785be173b491ae8a78faf5142892020 ] [immutable/offloaded.py: reduce use of get_serverid() a bit more warner@lothar.com**20110227011142 Ignore-this: b48acc1b2ae1b311da7f3ba4ffba38f ] [immutable/upload.py: reduce use of get_serverid() warner@lothar.com**20110227011138 Ignore-this: ffdd7ff32bca890782119a6e9f1495f6 ] [immutable/checker.py: remove some uses of s.get_serverid(), not all warner@lothar.com**20110227011134 Ignore-this: e480a37efa9e94e8016d826c492f626e ] [add remaining get_* methods to storage_client.Server, NoNetworkServer, and warner@lothar.com**20110227011132 Ignore-this: 6078279ddf42b179996a4b53bee8c421 MockIServer stubs ] [upload.py: rearrange _make_trackers a bit, no behavior changes warner@lothar.com**20110227011128 Ignore-this: 296d4819e2af452b107177aef6ebb40f ] [happinessutil.py: finally rename merge_peers to merge_servers warner@lothar.com**20110227011124 Ignore-this: c8cd381fea1dd888899cb71e4f86de6e ] [test_upload.py: factor out FakeServerTracker warner@lothar.com**20110227011120 Ignore-this: 6c182cba90e908221099472cc159325b ] [test_upload.py: server-vs-tracker cleanup warner@lothar.com**20110227011115 Ignore-this: 2915133be1a3ba456e8603885437e03 ] [happinessutil.py: server-vs-tracker cleanup warner@lothar.com**20110227011111 Ignore-this: b856c84033562d7d718cae7cb01085a9 ] [upload.py: more tracker-vs-server cleanup warner@lothar.com**20110227011107 Ignore-this: bb75ed2afef55e47c085b35def2de315 ] [upload.py: fix var names to avoid confusion between 'trackers' and 'servers' warner@lothar.com**20110227011103 Ignore-this: 5d5e3415b7d2732d92f42413c25d205d ] [refactor: s/peer/server/ in immutable/upload, happinessutil.py, test_upload warner@lothar.com**20110227011100 Ignore-this: 7ea858755cbe5896ac212a925840fe68 No behavioral changes, just updating variable/method names and log messages. The effects outside these three files should be minimal: some exception messages changed (to say "server" instead of "peer"), and some internal class names were changed. A few things still use "peer" to minimize external changes, like UploadResults.timings["peer_selection"] and happinessutil.merge_peers, which can be changed later. ] [storage_client.py: clean up test_add_server/test_add_descriptor, remove .test_servers warner@lothar.com**20110227011056 Ignore-this: efad933e78179d3d5fdcd6d1ef2b19cc ] [test_client.py, upload.py:: remove KiB/MiB/etc constants, and other dead code warner@lothar.com**20110227011051 Ignore-this: dc83c5794c2afc4f81e592f689c0dc2d ] [test: increase timeout on a network test because Francois's ARM machine hit that timeout zooko@zooko.com**20110317165909 Ignore-this: 380c345cdcbd196268ca5b65664ac85b I'm skeptical that the test was proceeding correctly but ran out of time. It seems more likely that it had gotten hung. But if we raise the timeout to an even more extravagant number then we can be even more certain that the test was never going to finish. ] [docs/configuration.rst: add a "Frontend Configuration" section Brian Warner **20110222014323 Ignore-this: 657018aa501fe4f0efef9851628444ca this points to docs/frontends/*.rst, which were previously underlinked ] [web/filenode.py: avoid calling req.finish() on closed HTTP connections. Closes #1366 "Brian Warner "**20110221061544 Ignore-this: 799d4de19933f2309b3c0c19a63bb888 ] [Add unit tests for cross_check_pkg_resources_versus_import, and a regression test for ref #1355. This requires a little refactoring to make it testable. david-sarah@jacaranda.org**20110221015817 Ignore-this: 51d181698f8c20d3aca58b057e9c475a ] [allmydata/__init__.py: .name was used in place of the correct .__name__ when printing an exception. Also, robustify string formatting by using %r instead of %s in some places. fixes #1355. david-sarah@jacaranda.org**20110221020125 Ignore-this: b0744ed58f161bf188e037bad077fc48 ] [Refactor StorageFarmBroker handling of servers Brian Warner **20110221015804 Ignore-this: 842144ed92f5717699b8f580eab32a51 Pass around IServer instance instead of (peerid, rref) tuple. Replace "descriptor" with "server". Other replacements: get_all_servers -> get_connected_servers/get_known_servers get_servers_for_index -> get_servers_for_psi (now returns IServers) This change still needs to be pushed further down: lots of code is now getting the IServer and then distributing (peerid, rref) internally. Instead, it ought to distribute the IServer internally and delay extracting a serverid or rref until the last moment. no_network.py was updated to retain parallelism. ] [TAG allmydata-tahoe-1.8.2 warner@lothar.com**20110131020101] Patch bundle hash: 48f5cedd889bc9d05be6343375b5908a8c42d000