From 51b1e5624af8553e16ab3b0686da261e4c2b5917 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 3 Sep 2021 14:04:10 -0400 Subject: [PATCH 1/7] Skeleton setting up the test infrastructure. --- src/allmydata/test/test_istorageserver.py | 38 +++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/allmydata/test/test_istorageserver.py diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py new file mode 100644 index 000000000..51d5da2c0 --- /dev/null +++ b/src/allmydata/test/test_istorageserver.py @@ -0,0 +1,38 @@ +""" +Tests for the ``IStorageServer`` interface. +""" + +from twisted.trial import unittest +from twisted.internet.defer import inlineCallbacks + +from allmydata.interfaces import IStorageServer +from .test_system import SystemTestMixin + + +class IStorageServerTestsMixin: + """ + Tests for ``IStorageServer``. + + ``self.storage_server`` is expected to provide ``IStorageServer``. + """ + @inlineCallbacks + def test_version(self): + yield self.storage_server.get_version() + + +class FoolscapIStorageServerTests( + SystemTestMixin, IStorageServerTestsMixin, unittest.TestCase +): + """Run tests on Foolscap version of ``IStorageServer.""" + + @inlineCallbacks + def setUp(self): + self.basedir = "test_istorageserver/{}/{}".format( + self.__class__.__name__, self._testMethodName + ) + yield SystemTestMixin.setUp(self) + yield self.set_up_nodes(1) + self.storage_server = next( + iter(self.clients[0].storage_broker.get_known_servers()) + ).get_storage_server() + self.assertTrue(IStorageServer.providedBy(self.storage_server)) From 855d02bef0587c7517417ac0528c1f65ae68111c Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 11:26:52 -0400 Subject: [PATCH 2/7] Start thinking about immutable tests. --- src/allmydata/test/test_istorageserver.py | 51 ++++++++++++++++++----- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index 51d5da2c0..b2229f467 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -1,38 +1,69 @@ """ Tests for the ``IStorageServer`` interface. + +Note that for performance, in the future we might want the same node to be +reused across tests, so each test should be careful to generate unique storage +indexes. """ -from twisted.trial import unittest -from twisted.internet.defer import inlineCallbacks +from twisted.internet.defer import inlineCallbacks, returnValue +from twisted.trial.unittest import TestCase from allmydata.interfaces import IStorageServer from .test_system import SystemTestMixin -class IStorageServerTestsMixin: +class IStorageServerSharedAPIsTestsMixin(object): """ - Tests for ``IStorageServer``. + Tests for ``IStorageServer``'s shared APIs. ``self.storage_server`` is expected to provide ``IStorageServer``. """ + @inlineCallbacks def test_version(self): + # TODO get_version() returns a dict-like thing with some of the + # expected fields. yield self.storage_server.get_version() -class FoolscapIStorageServerTests( - SystemTestMixin, IStorageServerTestsMixin, unittest.TestCase -): +class IStorageServerImmutableAPIsTestsMixin(object): + """ + Tests for ``IStorageServer``'s immutable APIs. + + ``self.storage_server`` is expected to provide ``IStorageServer``. + """ + + # TODO === allocate_buckets + RIBucketWriter === + # TODO allocate_buckets on a new storage index + # TODO allocate_buckets on existing bucket with same sharenums + # TODO allocate_buckets with smaller sharenums + # TODO allocate_buckets with larger sharenums + # TODO writes to bucket can happen in any order (write then read) + # TODO overlapping writes ignore already-written data (write then read) + + +class _FoolscapMixin(SystemTestMixin): """Run tests on Foolscap version of ``IStorageServer.""" @inlineCallbacks def setUp(self): - self.basedir = "test_istorageserver/{}/{}".format( - self.__class__.__name__, self._testMethodName - ) + self.basedir = "test_istorageserver/" + self.id() yield SystemTestMixin.setUp(self) yield self.set_up_nodes(1) self.storage_server = next( iter(self.clients[0].storage_broker.get_known_servers()) ).get_storage_server() self.assertTrue(IStorageServer.providedBy(self.storage_server)) + + +class FoolscapSharedAPIsTests( + _FoolscapMixin, IStorageServerSharedAPIsTestsMixin, TestCase +): + """Foolscap-specific tests for shared ``IStorageServer`` APIs.""" + + +class FoolscapImmutableAPIsTests( + _FoolscapMixin, IStorageServerImmutableAPIsTestsMixin, TestCase +): + """Foolscap-specific tests for immutable ``IStorageServer`` APIs.""" From 3bec2a480f8d17e8e22cbd23fac2b4611a786e36 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 12:20:27 -0400 Subject: [PATCH 3/7] Start on allocate_bucket tests. --- src/allmydata/test/test_istorageserver.py | 152 +++++++++++++++++++++- 1 file changed, 149 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index b2229f467..303b4b997 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -6,13 +6,68 @@ reused across tests, so each test should be careful to generate unique storage indexes. """ +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +from future.utils import PY2 + +if PY2: + from future.builtins import ( + filter, + map, + zip, + ascii, + chr, + hex, + input, + next, + oct, + open, + pow, + round, + super, + bytes, + dict, + list, + object, + range, + str, + max, + min, + ) # noqa: F401 + +from random import randrange +from unittest import expectedFailure + from twisted.internet.defer import inlineCallbacks, returnValue from twisted.trial.unittest import TestCase -from allmydata.interfaces import IStorageServer +from foolscap.api import Referenceable + +from allmydata.interfaces import IStorageServer, RIBucketWriter from .test_system import SystemTestMixin +def _randbytes(length): + # type: (int) -> bytes + """Return random bytes string of given length.""" + return bytes([randrange(0, 256) for _ in range(length)]) + + +def new_storage_index(): + # type: () -> bytes + """Return a new random storage index.""" + return _randbytes(16) + + +def new_secret(): + # type: () -> bytes + """Return a new random secret (for lease renewal or cancellation).""" + return _randbytes(32) + + class IStorageServerSharedAPIsTestsMixin(object): """ Tests for ``IStorageServer``'s shared APIs. @@ -35,13 +90,104 @@ class IStorageServerImmutableAPIsTestsMixin(object): """ # TODO === allocate_buckets + RIBucketWriter === - # TODO allocate_buckets on a new storage index - # TODO allocate_buckets on existing bucket with same sharenums + # DONE allocate_buckets on a new storage index + # PROG allocate_buckets on existing bucket with same sharenums # TODO allocate_buckets with smaller sharenums # TODO allocate_buckets with larger sharenums # TODO writes to bucket can happen in any order (write then read) # TODO overlapping writes ignore already-written data (write then read) + @inlineCallbacks + def test_allocate_buckets_new(self): + """ + allocate_buckets() with a new storage index returns the matching + shares. + """ + (already_got, allocated) = yield self.storage_server.allocate_buckets( + new_storage_index(), + new_secret(), + new_secret(), + set(range(5)), + 1024, + Referenceable(), + ) + self.assertEqual(already_got, set()) + self.assertEqual(allocated.keys(), set(range(5))) + # We validate the bucket objects' interface in a later test. + + @inlineCallbacks + def test_allocate_buckets_repeat(self): + """ + allocate_buckets() with the same storage index returns the same result, + because the shares have not been written to. + + This fails due to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793 + """ + si, renew_secret, cancel_secret = ( + new_storage_index(), + new_secret(), + new_secret(), + ) + (already_got, allocated) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + (already_got2, allocated2) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + self.assertEqual(already_got, already_got2) + self.assertEqual(allocated.keys(), allocated2.keys()) + + test_allocate_buckets_repeat.todo = ( + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" + ) + + @expectedFailure + @inlineCallbacks + def test_allocate_buckets_more_sharenums(self): + """ + allocate_buckets() with the same storage index but more sharenums + acknowledges the extra shares don't exist. + + Fails due to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793 + """ + si, renew_secret, cancel_secret = ( + new_storage_index(), + new_secret(), + new_secret(), + ) + yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + (already_got2, allocated2) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(7)), + 1024, + Referenceable(), + ) + self.assertEqual(already_got2, set()) # none were fully written + self.assertEqual(allocated2.keys(), set(range(7))) + + test_allocate_buckets_more_sharenums.todo = ( + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" + ) + class _FoolscapMixin(SystemTestMixin): """Run tests on Foolscap version of ``IStorageServer.""" From c1b1ed0dc34950b6931af575814e436ad9d0c8d1 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 13:52:34 -0400 Subject: [PATCH 4/7] More tests. --- src/allmydata/test/test_istorageserver.py | 105 ++++++++++++++++++++-- 1 file changed, 97 insertions(+), 8 deletions(-) diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index 303b4b997..92217debc 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -89,14 +89,6 @@ class IStorageServerImmutableAPIsTestsMixin(object): ``self.storage_server`` is expected to provide ``IStorageServer``. """ - # TODO === allocate_buckets + RIBucketWriter === - # DONE allocate_buckets on a new storage index - # PROG allocate_buckets on existing bucket with same sharenums - # TODO allocate_buckets with smaller sharenums - # TODO allocate_buckets with larger sharenums - # TODO writes to bucket can happen in any order (write then read) - # TODO overlapping writes ignore already-written data (write then read) - @inlineCallbacks def test_allocate_buckets_new(self): """ @@ -188,6 +180,103 @@ class IStorageServerImmutableAPIsTestsMixin(object): "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" ) + @inlineCallbacks + def test_written_shares_are_allocated(self): + """ + Shares that are fully written to show up as allocated in result from + ``IStoragServer.allocate_buckets()``. Partially-written or empty + shares don't. + """ + si, renew_secret, cancel_secret = ( + new_storage_index(), + new_secret(), + new_secret(), + ) + (_, allocated) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + + # Bucket 1 is fully written in one go. + yield allocated[1].callRemote("write", 0, b"1" * 1024) + yield allocated[1].callRemote("close") + + # Bucket 2 is fully written in two steps. + yield allocated[2].callRemote("write", 0, b"1" * 512) + yield allocated[2].callRemote("write", 512, b"2" * 512) + yield allocated[2].callRemote("close") + + # Bucket 0 has partial write. + yield allocated[0].callRemote("write", 0, b"1" * 512) + + (already_got, _) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + self.assertEqual(already_got, {1, 2}) + + @inlineCallbacks + def test_written_shares_are_readable(self): + """ + Shares that are fully written to can be read. + + 1. The result is not affected by the order in which writes + happened, only by their offsets. + + 2. When overlapping writes happen, the resulting read returns the + earliest written value. + """ + si, renew_secret, cancel_secret = ( + new_storage_index(), + new_secret(), + new_secret(), + ) + (_, allocated) = yield self.storage_server.allocate_buckets( + si, + renew_secret, + cancel_secret, + set(range(5)), + 1024, + Referenceable(), + ) + + # Bucket 1 is fully written in order + yield allocated[1].callRemote("write", 0, b"1" * 512) + yield allocated[1].callRemote("write", 512, b"2" * 512) + yield allocated[1].callRemote("close") + + # Bucket 2 is fully written in reverse. + yield allocated[2].callRemote("write", 512, b"4" * 512) + yield allocated[2].callRemote("write", 0, b"3" * 512) + yield allocated[2].callRemote("close") + + # Bucket 3 has an overlapping write. + yield allocated[3].callRemote("write", 0, b"5" * 20) + yield allocated[3].callRemote("write", 0, b"5" * 24) + yield allocated[3].callRemote("write", 24, b"6" * 1000) + yield allocated[3].callRemote("close") + + buckets = yield self.storage_server.get_buckets(si) + self.assertEqual(buckets.keys(), {1, 2, 3}) + + self.assertEqual( + (yield buckets[1].callRemote("read", 0, 1024)), b"1" * 512 + b"2" * 512 + ) + self.assertEqual( + (yield buckets[2].callRemote("read", 0, 1024)), b"3" * 512 + b"4" * 512 + ) + self.assertEqual( + (yield buckets[3].callRemote("read", 0, 1024)), b"5" * 24 + b"6" * 1000 + ) + class _FoolscapMixin(SystemTestMixin): """Run tests on Foolscap version of ``IStorageServer.""" From f3cb42d9a847ca8fa4a52fc9b69e2b4161a35071 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 13:52:51 -0400 Subject: [PATCH 5/7] News file. --- newsfragments/3784.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3784.minor diff --git a/newsfragments/3784.minor b/newsfragments/3784.minor new file mode 100644 index 000000000..e69de29bb From 44388037dfe034130829ca830ed2f5d39ff9f908 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 14:12:32 -0400 Subject: [PATCH 6/7] Flakes, and closer to passing on Python 2. --- src/allmydata/test/test_istorageserver.py | 44 ++++++----------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index 92217debc..68d7e2972 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -11,49 +11,28 @@ from __future__ import division from __future__ import print_function from __future__ import unicode_literals -from future.utils import PY2 +from future.utils import PY2, bchr if PY2: - from future.builtins import ( - filter, - map, - zip, - ascii, - chr, - hex, - input, - next, - oct, - open, - pow, - round, - super, - bytes, - dict, - list, - object, - range, - str, - max, - min, - ) # noqa: F401 + # fmt: off + from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 + # fmt: on from random import randrange -from unittest import expectedFailure -from twisted.internet.defer import inlineCallbacks, returnValue +from twisted.internet.defer import inlineCallbacks from twisted.trial.unittest import TestCase from foolscap.api import Referenceable -from allmydata.interfaces import IStorageServer, RIBucketWriter +from allmydata.interfaces import IStorageServer from .test_system import SystemTestMixin def _randbytes(length): # type: (int) -> bytes """Return random bytes string of given length.""" - return bytes([randrange(0, 256) for _ in range(length)]) + return b"".join([bchr(randrange(0, 256)) for _ in range(length)]) def new_storage_index(): @@ -104,7 +83,7 @@ class IStorageServerImmutableAPIsTestsMixin(object): Referenceable(), ) self.assertEqual(already_got, set()) - self.assertEqual(allocated.keys(), set(range(5))) + self.assertEqual(set(allocated.keys()), set(range(5))) # We validate the bucket objects' interface in a later test. @inlineCallbacks @@ -137,13 +116,12 @@ class IStorageServerImmutableAPIsTestsMixin(object): Referenceable(), ) self.assertEqual(already_got, already_got2) - self.assertEqual(allocated.keys(), allocated2.keys()) + self.assertEqual(set(allocated.keys()), set(allocated2.keys())) test_allocate_buckets_repeat.todo = ( "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" ) - @expectedFailure @inlineCallbacks def test_allocate_buckets_more_sharenums(self): """ @@ -174,7 +152,7 @@ class IStorageServerImmutableAPIsTestsMixin(object): Referenceable(), ) self.assertEqual(already_got2, set()) # none were fully written - self.assertEqual(allocated2.keys(), set(range(7))) + self.assertEqual(set(allocated2.keys()), set(range(7))) test_allocate_buckets_more_sharenums.todo = ( "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" @@ -265,7 +243,7 @@ class IStorageServerImmutableAPIsTestsMixin(object): yield allocated[3].callRemote("close") buckets = yield self.storage_server.get_buckets(si) - self.assertEqual(buckets.keys(), {1, 2, 3}) + self.assertEqual(set(buckets.keys()), {1, 2, 3}) self.assertEqual( (yield buckets[1].callRemote("read", 0, 1024)), b"1" * 512 + b"2" * 512 From a2d54aa8bc4cfc7aaf40a5c3f29246d51606fce9 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 8 Sep 2021 14:14:36 -0400 Subject: [PATCH 7/7] .todo isn't working on Python 2 for some reason. --- src/allmydata/test/test_istorageserver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index 68d7e2972..75fbe42e2 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -118,7 +118,7 @@ class IStorageServerImmutableAPIsTestsMixin(object): self.assertEqual(already_got, already_got2) self.assertEqual(set(allocated.keys()), set(allocated2.keys())) - test_allocate_buckets_repeat.todo = ( + test_allocate_buckets_repeat.skip = ( "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" ) @@ -154,7 +154,7 @@ class IStorageServerImmutableAPIsTestsMixin(object): self.assertEqual(already_got2, set()) # none were fully written self.assertEqual(set(allocated2.keys()), set(range(7))) - test_allocate_buckets_more_sharenums.todo = ( + test_allocate_buckets_more_sharenums.skip = ( "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793" )