From af86066eab671b3f6582d20cea3dfd4a71ac358f Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 9 Jul 2020 13:48:33 -0400 Subject: [PATCH 01/20] Delete some dead code. --- newsfragments/3340.other | 0 src/allmydata/test/common_util.py | 33 ---------------- src/allmydata/test/test_client.py | 2 +- src/allmydata/util/eliotutil.py | 66 ------------------------------- src/allmydata/util/fileutil.py | 54 ------------------------- src/allmydata/util/rrefutil.py | 10 +---- 6 files changed, 3 insertions(+), 162 deletions(-) create mode 100644 newsfragments/3340.other diff --git a/newsfragments/3340.other b/newsfragments/3340.other new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py index f9e0f2b01..025459961 100644 --- a/src/allmydata/test/common_util.py +++ b/src/allmydata/test/common_util.py @@ -89,39 +89,6 @@ class ReallyEqualMixin(object): self.assertEqual(type(a), type(b), "a :: %r, b :: %r, %r" % (a, b, msg)) -class NonASCIIPathMixin(object): - def mkdir_nonascii(self, dirpath): - # Kludge to work around the fact that buildbot can't remove a directory tree that has - # any non-ASCII directory names on Windows. (#1472) - if sys.platform == "win32": - def _cleanup(): - try: - fileutil.rm_dir(dirpath) - finally: - if os.path.exists(dirpath): - msg = ("We were unable to delete a non-ASCII directory %r created by the test. " - "This is liable to cause failures on future builds." % (dirpath,)) - print(msg) - log.err(msg) - self.addCleanup(_cleanup) - os.mkdir(dirpath) - - def unicode_or_fallback(self, unicode_name, fallback_name, io_as_well=False): - if not unicode_platform(): - try: - unicode_name.encode(get_filesystem_encoding()) - except UnicodeEncodeError: - return fallback_name - - if io_as_well: - try: - unicode_name.encode(get_io_encoding()) - except UnicodeEncodeError: - return fallback_name - - return unicode_name - - class SignalMixin(object): # This class is necessary for any code which wants to use Processes # outside the usual reactor.run() environment. It is copied from diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index 41a44d5a6..b188008fd 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -83,7 +83,7 @@ BASECONFIG_I = ("[client]\n" "introducer.furl = %s\n" ) -class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.TestCase): +class Basic(testutil.ReallyEqualMixin, unittest.TestCase): def test_loadable(self): basedir = "test_client.Basic.test_loadable" os.mkdir(basedir) diff --git a/src/allmydata/util/eliotutil.py b/src/allmydata/util/eliotutil.py index f6f40945d..c4805ff3c 100644 --- a/src/allmydata/util/eliotutil.py +++ b/src/allmydata/util/eliotutil.py @@ -14,8 +14,6 @@ __all__ = [ "eliot_logging_service", "opt_eliot_destination", "opt_help_eliot_destinations", - "validateInstanceOf", - "validateSetMembership", ] from sys import ( @@ -75,23 +73,6 @@ from twisted.internet.defer import ( ) from twisted.application.service import Service -def validateInstanceOf(t): - """ - Return an Eliot validator that requires values to be instances of ``t``. - """ - def validator(v): - if not isinstance(v, t): - raise ValidationError("{} not an instance of {}".format(v, t)) - return validator - -def validateSetMembership(s): - """ - Return an Eliot validator that requires values to be elements of ``s``. - """ - def validator(v): - if v not in s: - raise ValidationError("{} not in {}".format(v, s)) - return validator def eliot_logging_service(reactor, destinations): """ @@ -246,53 +227,6 @@ class _DestinationParser(object): else: return parser(kind, args) - def _get_arg(self, arg_name, default, arg_list): - return dict( - arg.split(u"=", 1) - for arg - in arg_list - ).get( - arg_name, - default, - ) - - def _parse_file(self, kind, arg_text): - # Reserve the possibility of an escape character in the future. \ is - # the standard choice but it's the path separator on Windows which - # pretty much ruins it in this context. Most other symbols already - # have some shell-assigned meaning which makes them treacherous to use - # in a CLI interface. Eliminating all such dangerous symbols leaves - # approximately @. - if u"@" in arg_text: - raise ValueError( - u"Unsupported escape character (@) in destination text ({!r}).".format(arg_text), - ) - arg_list = arg_text.split(u",") - path_name = arg_list.pop(0) - if path_name == "-": - get_file = lambda: stdout - else: - path = FilePath(path_name) - rotate_length = int(self._get_arg( - u"rotate_length", - 1024 * 1024 * 1024, - arg_list, - )) - max_rotated_files = int(self._get_arg( - u"max_rotated_files", - 10, - arg_list, - )) - def get_file(): - path.parent().makedirs(ignoreExistingDirectory=True) - return LogFile( - path.basename(), - path.dirname(), - rotateLength=rotate_length, - maxRotatedFiles=max_rotated_files, - ) - return lambda reactor: FileDestination(get_file()) - _parse_destination_description = _DestinationParser().parse diff --git a/src/allmydata/util/fileutil.py b/src/allmydata/util/fileutil.py index 162852c38..16adf2f43 100644 --- a/src/allmydata/util/fileutil.py +++ b/src/allmydata/util/fileutil.py @@ -538,60 +538,6 @@ def get_available_space(whichdir, reserved_space): return 0 -if sys.platform == "win32": - # - CreateFileW = WINFUNCTYPE( - HANDLE, LPCWSTR, DWORD, DWORD, LPVOID, DWORD, DWORD, HANDLE, - use_last_error=True - )(("CreateFileW", windll.kernel32)) - - GENERIC_WRITE = 0x40000000 - FILE_SHARE_READ = 0x00000001 - FILE_SHARE_WRITE = 0x00000002 - OPEN_EXISTING = 3 - INVALID_HANDLE_VALUE = 0xFFFFFFFF - - # - FlushFileBuffers = WINFUNCTYPE( - BOOL, HANDLE, - use_last_error=True - )(("FlushFileBuffers", windll.kernel32)) - - # - CloseHandle = WINFUNCTYPE( - BOOL, HANDLE, - use_last_error=True - )(("CloseHandle", windll.kernel32)) - - # - def flush_volume(path): - abspath = os.path.realpath(path) - if abspath.startswith("\\\\?\\"): - abspath = abspath[4 :] - drive = os.path.splitdrive(abspath)[0] - - print("flushing %r" % (drive,)) - hVolume = CreateFileW(u"\\\\.\\" + drive, - GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, - None, - OPEN_EXISTING, - 0, - None - ) - if hVolume == INVALID_HANDLE_VALUE: - raise WinError(get_last_error()) - - if FlushFileBuffers(hVolume) == 0: - raise WinError(get_last_error()) - - CloseHandle(hVolume) -else: - def flush_volume(path): - # use sync()? - pass - - class ConflictError(Exception): pass diff --git a/src/allmydata/util/rrefutil.py b/src/allmydata/util/rrefutil.py index 0dde651d3..40e921507 100644 --- a/src/allmydata/util/rrefutil.py +++ b/src/allmydata/util/rrefutil.py @@ -1,7 +1,7 @@ from twisted.internet import address -from foolscap.api import Violation, RemoteException, DeadReferenceError, \ - SturdyRef +from foolscap.api import Violation, RemoteException, SturdyRef + def add_version_to_remote_reference(rref, default): """I try to add a .version attribute to the given RemoteReference. I call @@ -19,12 +19,6 @@ def add_version_to_remote_reference(rref, default): d.addCallbacks(_got_version, _no_get_version) return d -def trap_and_discard(f, *errorTypes): - f.trap(*errorTypes) - -def trap_deadref(f): - return trap_and_discard(f, DeadReferenceError) - def connection_hints_for_furl(furl): hints = [] From e108ecb4b0fef5a6f5e2a1ef6cfea53e5f951234 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 9 Jul 2020 13:56:25 -0400 Subject: [PATCH 02/20] Fix lint. --- src/allmydata/test/common_util.py | 3 +-- src/allmydata/util/eliotutil.py | 13 ------------- src/allmydata/util/fileutil.py | 2 +- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py index 025459961..bf44785a5 100644 --- a/src/allmydata/test/common_util.py +++ b/src/allmydata/test/common_util.py @@ -1,6 +1,6 @@ from __future__ import print_function -import os, signal, sys, time +import os, signal, time from random import randrange from six.moves import StringIO @@ -8,7 +8,6 @@ from twisted.internet import reactor, defer from twisted.python import failure from twisted.trial import unittest -from allmydata.util import fileutil, log from ..util.assertutil import precondition from allmydata.util.encodingutil import (unicode_platform, get_filesystem_encoding, get_io_encoding) diff --git a/src/allmydata/util/eliotutil.py b/src/allmydata/util/eliotutil.py index c4805ff3c..14c867ed2 100644 --- a/src/allmydata/util/eliotutil.py +++ b/src/allmydata/util/eliotutil.py @@ -16,9 +16,6 @@ __all__ = [ "opt_help_eliot_destinations", ] -from sys import ( - stdout, -) from functools import wraps from logging import ( INFO, @@ -40,15 +37,11 @@ from attr.validators import ( from eliot import ( ILogger, Message, - FileDestination, add_destinations, remove_destination, write_traceback, start_action, ) -from eliot._validation import ( - ValidationError, -) from eliot.twisted import ( DeferredContext, inline_callbacks, @@ -57,12 +50,6 @@ from eliot.twisted import ( from twisted.python.usage import ( UsageError, ) -from twisted.python.filepath import ( - FilePath, -) -from twisted.python.logfile import ( - LogFile, -) from twisted.logger import ( ILogObserver, eventAsJSON, diff --git a/src/allmydata/util/fileutil.py b/src/allmydata/util/fileutil.py index 16adf2f43..269a8a356 100644 --- a/src/allmydata/util/fileutil.py +++ b/src/allmydata/util/fileutil.py @@ -12,7 +12,7 @@ from errno import ENOENT if sys.platform == "win32": from ctypes import WINFUNCTYPE, WinError, windll, POINTER, byref, c_ulonglong, \ create_unicode_buffer, get_last_error - from ctypes.wintypes import BOOL, DWORD, LPCWSTR, LPWSTR, LPVOID, HANDLE + from ctypes.wintypes import BOOL, DWORD, LPCWSTR, LPWSTR, LPVOID from twisted.python import log From 1a3a1ffcaa8c8cd71d156cd4bc78110f4359b303 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 13 Jul 2020 14:33:13 -0400 Subject: [PATCH 03/20] Restore eliot code. --- src/allmydata/util/eliotutil.py | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/allmydata/util/eliotutil.py b/src/allmydata/util/eliotutil.py index 14c867ed2..f6f40945d 100644 --- a/src/allmydata/util/eliotutil.py +++ b/src/allmydata/util/eliotutil.py @@ -14,8 +14,13 @@ __all__ = [ "eliot_logging_service", "opt_eliot_destination", "opt_help_eliot_destinations", + "validateInstanceOf", + "validateSetMembership", ] +from sys import ( + stdout, +) from functools import wraps from logging import ( INFO, @@ -37,11 +42,15 @@ from attr.validators import ( from eliot import ( ILogger, Message, + FileDestination, add_destinations, remove_destination, write_traceback, start_action, ) +from eliot._validation import ( + ValidationError, +) from eliot.twisted import ( DeferredContext, inline_callbacks, @@ -50,6 +59,12 @@ from eliot.twisted import ( from twisted.python.usage import ( UsageError, ) +from twisted.python.filepath import ( + FilePath, +) +from twisted.python.logfile import ( + LogFile, +) from twisted.logger import ( ILogObserver, eventAsJSON, @@ -60,6 +75,23 @@ from twisted.internet.defer import ( ) from twisted.application.service import Service +def validateInstanceOf(t): + """ + Return an Eliot validator that requires values to be instances of ``t``. + """ + def validator(v): + if not isinstance(v, t): + raise ValidationError("{} not an instance of {}".format(v, t)) + return validator + +def validateSetMembership(s): + """ + Return an Eliot validator that requires values to be elements of ``s``. + """ + def validator(v): + if v not in s: + raise ValidationError("{} not in {}".format(v, s)) + return validator def eliot_logging_service(reactor, destinations): """ @@ -214,6 +246,53 @@ class _DestinationParser(object): else: return parser(kind, args) + def _get_arg(self, arg_name, default, arg_list): + return dict( + arg.split(u"=", 1) + for arg + in arg_list + ).get( + arg_name, + default, + ) + + def _parse_file(self, kind, arg_text): + # Reserve the possibility of an escape character in the future. \ is + # the standard choice but it's the path separator on Windows which + # pretty much ruins it in this context. Most other symbols already + # have some shell-assigned meaning which makes them treacherous to use + # in a CLI interface. Eliminating all such dangerous symbols leaves + # approximately @. + if u"@" in arg_text: + raise ValueError( + u"Unsupported escape character (@) in destination text ({!r}).".format(arg_text), + ) + arg_list = arg_text.split(u",") + path_name = arg_list.pop(0) + if path_name == "-": + get_file = lambda: stdout + else: + path = FilePath(path_name) + rotate_length = int(self._get_arg( + u"rotate_length", + 1024 * 1024 * 1024, + arg_list, + )) + max_rotated_files = int(self._get_arg( + u"max_rotated_files", + 10, + arg_list, + )) + def get_file(): + path.parent().makedirs(ignoreExistingDirectory=True) + return LogFile( + path.basename(), + path.dirname(), + rotateLength=rotate_length, + maxRotatedFiles=max_rotated_files, + ) + return lambda reactor: FileDestination(get_file()) + _parse_destination_description = _DestinationParser().parse From 73c40e8d2aabe8f6b7dde6aa1ef20be542a50349 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 26 Jun 2020 07:32:53 -0400 Subject: [PATCH 04/20] Minimally add py36 to CI --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 44b583baa..259fb199c 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ twisted = 1 [tox] -envlist = {py27,pypy27}{-coverage,} +envlist = {py27,pypy27,py36}{-coverage,} minversion = 2.4 [testenv] From 2eab253b0f6b909ef7ba54ff10c48fb5e58c7eda Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 26 Jun 2020 07:50:42 -0400 Subject: [PATCH 05/20] Change python_requires in setup.py --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 047d08030..0742a7ce8 100644 --- a/setup.py +++ b/setup.py @@ -353,7 +353,9 @@ setup(name="tahoe-lafs", # also set in __init__.py package_dir = {'':'src'}, packages=find_packages('src') + ['allmydata.test.plugins'], classifiers=trove_classifiers, - python_requires="<3.0", + # We support Python 2.7, and we're working on support for 3.6 (the + # highest version that PyPy currently supports). + python_requires=">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*, <3.7", install_requires=install_requires, extras_require={ # Duplicate the Twisted pywin32 dependency here. See From fec1f73bfadaea5ab0cfff685ba13923856e5242 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 1 Jul 2020 07:18:33 -0400 Subject: [PATCH 06/20] Ignore eliot.log, some testing artifact --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ee3b02b8f..caf0f2e87 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ zope.interface-*.egg /docs/_build/ /coverage.xml /.hypothesis/ +/eliot.log # This is the plaintext of the private environment needed for some CircleCI # operations. It's never supposed to be checked in. From 329bfe05f942d08184bad3a303d049c82de4459a Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 1 Jul 2020 07:49:23 -0400 Subject: [PATCH 07/20] Make a crucial relative import explicit --- src/allmydata/version_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/version_checks.py b/src/allmydata/version_checks.py index aa42c2eee..7092a422b 100644 --- a/src/allmydata/version_checks.py +++ b/src/allmydata/version_checks.py @@ -297,7 +297,7 @@ def _get_platform(): def _get_package_versions_and_locations(): import warnings - from _auto_deps import package_imports, global_deprecation_messages, deprecation_messages, \ + from ._auto_deps import package_imports, global_deprecation_messages, deprecation_messages, \ runtime_warning_messages, warning_imports, ignorable def package_dir(srcfile): From 5a2ee5387a888c60f5c167edb35d48814408f060 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Wed, 1 Jul 2020 07:59:39 -0400 Subject: [PATCH 08/20] Bump txi2p to unreleased version that supports Py3 The plan is to keep i2p support around, in the hopes that upstream libs (txi2p, foolscap) are ported to Python 3 by the time we _really_ need them. --- setup.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 0742a7ce8..f7f7b14f5 100644 --- a/setup.py +++ b/setup.py @@ -141,8 +141,9 @@ tor_requires = [ ] i2p_requires = [ - # See the comment in tor_requires. - "txi2p >= 0.3.2", + # txi2p has Python 3 support, but it's unreleased: https://github.com/str4d/txi2p/issues/10. + # Also see the comment in tor_requires. + "txi2p @ git+https://github.com/str4d/txi2p@0611b9a86172cb70d2f5e415a88eee9f230590b3#egg=txi2p", ] if len(sys.argv) > 1 and sys.argv[1] == '--fakedependency': From 076c73d22f2179ab01f9087e10294b685f63790e Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Thu, 2 Jul 2020 07:35:31 -0400 Subject: [PATCH 09/20] Turn on py36 at Travis Drops the old py35 job, per @exarkun: https://github.com/tahoe-lafs/tahoe-lafs/pull/732#discussion_r449615599 Also bumps Ubuntu at Travis so we can get 3.8 eventually. I first went to 3.8 before dropping back to 3.6 as our initial target. Trusty on Travis does include 3.6, but since we want 3.8 "pretty soon," and the OS bump ended up being tricky (see below), let's go ahead and keep the OS bump. Xenial (16.04) is the current default at Travis, and it does have 3.8 available: https://docs.travis-ci.com/user/languages/python/#python-versions The tricky bug is that different versions of virtualenv have different seeding algorithms: https://discuss.python.org/t/-/4146). CI puts us several layers deep in virtualenv-ception and I didn't fully unravel the whole thing, but starting with a modern virtualenv seems to work around the issue. --- .travis.yml | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 490e37b7d..dec435604 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: false language: python cache: pip -dist: trusty +dist: xenial before_cache: - rm -f $HOME/.cache/pip/log/debug.log git: @@ -16,19 +16,15 @@ install: - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then export PATH=$HOME/Library/Python/2.7/bin:$PATH; fi - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then wget https://bootstrap.pypa.io/get-pip.py && sudo python ./get-pip.py; fi - pip list - - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then pip install --user --upgrade codecov tox setuptools; fi - - if [ "${TRAVIS_OS_NAME}" = "linux" ]; then pip install --upgrade codecov tox setuptools; fi + - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then pip install --user --upgrade codecov tox setuptools virtualenv; fi + - if [ "${TRAVIS_OS_NAME}" = "linux" ]; then pip install --upgrade codecov tox setuptools virtualenv; fi - echo $PATH; which python; which pip; which tox - python misc/build_helpers/show-tool-versions.py script: - | set -eo pipefail - if [ "${T}" = "py35" ]; then - python3 -m compileall -f -x tahoe-depgraph.py . - else - tox -e ${T} - fi + tox -e ${T} # To verify that the resultant PyInstaller-generated binary executes # cleanly (i.e., that it terminates with an exit code of 0 and isn't # failing due to import/packaging-related errors, etc.). @@ -69,9 +65,8 @@ matrix: python: '2.7' env: T=pyinstaller LANG=en_US.UTF-8 language: generic # "python" is not available on OS-X - # this is a "lint" job that checks for python3 compatibility - os: linux - python: '3.5' - env: T=py35 + python: '3.6' + env: T=py36 fast_finish: true From b47b4a468b46880d275f1da97fdb3657e2cd7900 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 3 Jul 2020 07:31:43 -0400 Subject: [PATCH 10/20] Prevent Nevow from blocking us Per https://github.com/tahoe-lafs/tahoe-lafs/pull/732#issuecomment-653059972 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 259fb199c..af45f5da8 100644 --- a/tox.ini +++ b/tox.ini @@ -45,8 +45,8 @@ usedevelop = False # tests. extras = test commands = - tahoe --version trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors} {posargs:allmydata} + tahoe --version [testenv:integration] setenv = From d25c8b1a25c8f3b9a117ac04c46c0e098668867d Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 3 Jul 2020 17:35:48 -0400 Subject: [PATCH 11/20] Start ratcheting up passing tests under Python 3 --- .gitignore | 1 + misc/python3/passing | 13 ++ misc/python3/ratchet.py | 411 ++++++++++++++++++++++++++++++++++++++++ misc/python3/ratchet.sh | 31 +++ setup.py | 1 + tox.ini | 5 + 6 files changed, 462 insertions(+) create mode 100644 misc/python3/passing create mode 100755 misc/python3/ratchet.py create mode 100755 misc/python3/ratchet.sh diff --git a/.gitignore b/.gitignore index caf0f2e87..b33c1aab8 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,7 @@ zope.interface-*.egg /coverage.xml /.hypothesis/ /eliot.log +/misc/python3/results.xml # This is the plaintext of the private environment needed for some CircleCI # operations. It's never supposed to be checked in. diff --git a/misc/python3/passing b/misc/python3/passing new file mode 100644 index 000000000..e659c95b6 --- /dev/null +++ b/misc/python3/passing @@ -0,0 +1,13 @@ +allmydata.test.mutable.test_exceptions.Exceptions.test_repr +allmydata.test.test_deferredutil.DeferredUtilTests.test_failure +allmydata.test.test_deferredutil.DeferredUtilTests.test_gather_results +allmydata.test.test_deferredutil.DeferredUtilTests.test_success +allmydata.test.test_deferredutil.DeferredUtilTests.test_wait_for_delayed_calls +allmydata.test.test_humanreadable.HumanReadable.test_repr +allmydata.test.test_observer.Observer.test_lazy_oneshot +allmydata.test.test_observer.Observer.test_observerlist +allmydata.test.test_observer.Observer.test_oneshot +allmydata.test.test_observer.Observer.test_oneshot_fireagain +allmydata.test.test_python3.Python3PortingEffortTests.test_finished_porting +allmydata.test.test_python3.Python3PortingEffortTests.test_ported_modules_distinct +allmydata.test.test_python3.Python3PortingEffortTests.test_ported_modules_exist diff --git a/misc/python3/ratchet.py b/misc/python3/ratchet.py new file mode 100755 index 000000000..136fdcf4c --- /dev/null +++ b/misc/python3/ratchet.py @@ -0,0 +1,411 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +'''Ratchet up passing tests, or ratchet down failing tests. + +Usage: + + ratchet.py <"up" or "down"> + +This script helps when you expect a large test suite to fail spectactularly in +some environment, and you want to gradually improve the situation with minimal +impact to forward development of the same codebase for other environments. The +initial and primary usecase is porting from Python 2 to Python 3. + +The idea is to emit JUnit XML from your test runner, and then invoke ratchet.py +to consume this XML output and operate on a so-called "tracking" file. When +ratcheting up passing tests, the tracking file will contain a list of tests, +one per line, that passed. When ratching down, the tracking file contains a +list of failing tests. On each subsequent run, ratchet.py will compare the +prior results in the tracking file with the new results in the XML, and will +report on both welcome and unwelcome changes. It will modify the tracking file +in the case of welcome changes, and therein lies the ratcheting. + +The exit codes are: + + 0 - no changes observed + 1 - changes observed, whether welcome or unwelcome + 2 - invocation error + +Be sure to call as `./ratchet.py` and not `python ratchet.py` if you want to +see our exit code instead of Python's. + +If does not exist, you'll get a FileNotFoundError: + + >>> _test('up', None, None) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + FileNotFoundError: ... + +If does not exist, that's fine: + + >>> _test('up', '1', None) + Some tests not required to pass did: + c0.t + Conveniently, they have been added to `` for you. Perhaps commit that? + Eep! 0 test(s) were required to pass, but instead 1 did. 🐭 + +Same if you're ratcheting down: + + >>> _test('down', '1', None) + All and only tests expected to fail did. 💃 + +If the test run has the same output as last time, it's all good: + + >>> _test('up', '01001110', '01001110') + All and only tests required to pass did. 💃 + + >>> _test('down', '01001110', '10110001') + All and only tests expected to fail did. 💃 + +If there's a welcome change, that's noted: + + >>> _test('up', '0101', '0100') + Some tests not required to pass did: + c3.t + Conveniently, they have been added to `` for you. Perhaps commit that? + Eep! 1 test(s) were required to pass, but instead 2 did. 🐭 + + >>> _test('down', '0011', '1110') + Some tests expected to fail didn't: + c2.t + Conveniently, they have been removed from `` for you. Perhaps commit that? + Eep! 3 test(s) were expected to fail, but instead 2 did. 🐭 + +And if there is an unwelcome change, that is noted as well: + + >>> _test('up', '1101', '1111') + Some tests required to pass didn't: + c2.t + Eep! 4 test(s) were required to pass, but instead 3 did. 🐭 + + >>> _test('down', '0000', '1101') + Some tests not expected to fail did: + c2.t + Eep! 3 test(s) were expected to fail, but instead 4 did. 🐭 + +And if there are both welcome and unwelcome changes, they are both noted: + + >>> _test('up', '1101', '1011') + Some tests not required to pass did: + c1.t + Conveniently, they have been added to `` for you. Perhaps commit that? + Some tests required to pass didn't: + c2.t + Eep! 3 test(s) were required to pass, but instead 3 did. 🐭 + + >>> _test('down', '0100', '1100') + Some tests not expected to fail did: + c2.t + c3.t + Some tests expected to fail didn't: + c1.t + Conveniently, they have been removed from `` for you. Perhaps commit that? + Eep! 2 test(s) were expected to fail, but instead 3 did. 🐭 + + +To test ratchet.py itself: + + python3 -m doctest ratchet.py + +''' +from __future__ import absolute_import, division, print_function, unicode_literals + +import io +import os +import re +import sys +import tempfile +import xml.etree.ElementTree as Etree + + +class JUnitXMLFile(object): + '''Represent a file containing test results in JUnit XML format. + + >>> eg = _mktemp_junitxml('0100111') + >>> results = JUnitXMLFile(eg.name).parse() + >>> results.failed + ['c0.t', 'c2.t', 'c3.t'] + >>> results.passed + ['c1.t', 'c4.t', 'c5.t', 'c6.t'] + + ''' + + def __init__(self, filepath): + self.filepath = filepath + self.failed = [] + self.failed_aggregates = {} + self.stderr_output = [] + self.passed = [] + self._tree = None + + def parse(self): + if self._tree: + raise RuntimeError('already parsed') + self._tree = Etree.parse(self.filepath) + for testcase in self._tree.findall('testcase'): + self.process_testcase(testcase) + return self + + def process_testcase(self, case): + key = self.case_key(case) + + # look at children but throw away stderr output + nonpassing = [c for c in case if not c.tag == 'system-err'] + n = len(nonpassing) + if n > 1: + raise RuntimeError(f'multiple results for {key}: {nonpassing}') + elif n == 1: + result = nonpassing.pop() + self.failed.append(key) + message = result.get('message') + self.failed_aggregates.setdefault(message, []).append(key) + else: + self.passed.append(key) + + @staticmethod + def case_key(case): + return f'{case.get("classname")}.{case.get("name")}' + + def report(self, details=False): + for k, v in sorted( + self.failed_aggregates.items(), + key = lambda i: len(i[1]), + reverse=True): + print(f'# {k}') + for t in v: + print(f' - {t}') + + +def load_previous_results(txt): + try: + previous_results = open(txt).read() + except FileNotFoundError: + previous_results = '' + parsed = set() + for line in previous_results.splitlines(): + if not line or line.startswith('#'): + continue + parsed.add(line) + return parsed + + +def print_tests(tests): + for test in sorted(tests): + print(' ', test) + + +def ratchet_up_passing(tracking_path, tests): + try: + old = set(open(tracking_path, 'r')) + except FileNotFoundError: + old = set() + new = set(t + '\n' for t in tests) + merged = sorted(old | new) + open(tracking_path, 'w+').writelines(merged) + + +def ratchet_down_failing(tracking_path, tests): + new = set(t + '\n' for t in tests) + open(tracking_path, 'w+').writelines(sorted(new)) + + +def main(direction, junitxml_path, tracking_path): + '''Takes a string indicating which direction to ratchet, "up" or "down," + and two paths, one to test-runner output in JUnit XML format, the other to + a file tracking test results (one test case dotted name per line). Walk the + former looking for the latter, and react appropriately. + + >>> inp = _mktemp_junitxml('0100111') + >>> out = _mktemp_tracking('0000000') + >>> _test_main('up', inp.name, out.name) + Some tests not required to pass did: + c1.t + c4.t + c5.t + c6.t + Conveniently, they have been added to `` for you. Perhaps commit that? + Eep! 0 test(s) were required to pass, but instead 4 did. 🐭 + + ''' + + results = JUnitXMLFile(junitxml_path).parse() + + if tracking_path == '...': + results.report() + return + + previous = load_previous_results(tracking_path) + current = set(results.passed if direction == 'up' else results.failed) + + subjunctive = {'up': 'required to pass', 'down': 'expected to fail'}[direction] + ratchet = None + + too_many = current - previous + if too_many: + print(f'Some tests not {subjunctive} did:') + print_tests(too_many) + if direction == 'up': + # Too many passing tests is good -- let's do more of those! + ratchet_up_passing(tracking_path, current) + print(f'Conveniently, they have been added to `{tracking_path}` for you. Perhaps commit that?') + + not_enough = previous - current + if not_enough: + print(f'Some tests {subjunctive} didn\'t:') + print_tests(not_enough) + if direction == 'down': + # Not enough failing tests is good -- let's do more of those! + ratchet_down_failing(tracking_path, current) + print(f'Conveniently, they have been removed from `{tracking_path}` for you. Perhaps commit that?') + + if too_many or not_enough: + print(f'Eep! {len(previous)} test(s) were {subjunctive}, but instead {len(current)} did. 🐭') + return 1 + + print(f'All and only tests {subjunctive} did. 💃') + return 0 + + +# When called as an executable ... + +if __name__ == '__main__': + try: + direction, junitxml_path, tracking_path = sys.argv[1:4] + if direction not in ('up', 'down'): + raise ValueError + except ValueError: + doc = '\n'.join(__doc__.splitlines()[:6]) + doc = re.sub(' ratchet.py', f' {sys.argv[0]}', doc) + print(doc, file=sys.stderr) + exit_code = 2 + else: + exit_code = main(direction, junitxml_path, tracking_path) + sys.exit(exit_code) + + +# Helpers for when called under doctest ... + +def _test(*a): + return _test_main(*_mk(*a)) + + +def _test_main(direction, junitxml, tracking): + '''Takes a string 'up' or 'down' and paths to (or open file objects for) + the JUnit XML and tracking files to use for this test run. Captures and + emits stdout (slightly modified) for inspection via doctest.''' + junitxml_path = junitxml.name if hasattr(junitxml, 'name') else junitxml + tracking_path = tracking.name if hasattr(tracking, 'name') else tracking + + old_stdout = sys.stdout + sys.stdout = io.StringIO() + try: + main(direction, junitxml_path, tracking_path) + finally: + sys.stdout.seek(0) + out = sys.stdout.read() + out = re.sub('`.*?`', '``', out).strip() + sys.stdout = old_stdout + print(out) + + +class _PotentialFile(object): + '''Represent a file that we are able to create but which doesn't exist yet, + and which, if we create it, will be automatically torn down when the test + run is over.''' + + def __init__(self, filename): + self.d = tempfile.TemporaryDirectory() + self.name = os.path.join(self.d.name, filename) + + +def _mk(direction, spec_junitxml, spec_tracking): + '''Takes a string 'up' or 'down' and two bit strings specifying the state + of the JUnit XML results file and the tracking file to set up for this test + case. Returns the direction (unharmed) and two file-ish objects. + + If a spec string is None the corresponding return value will be a + _PotentialFile object, which has a .name attribute (like a true file + object) that points to a file that does not exist, but could. + + The reason not to simply return the path in all cases is that the file + objects are actually temporary file objects that destroy the underlying + file when they go out of scope, and we want to keep the underlying file + around until the end of the test run.''' + + if None not in(spec_junitxml, spec_tracking): + if len(spec_junitxml) != len(spec_tracking): + raise ValueError('if both given, must be the same length: `{spec_junitxml}` and `{spec_tracking}`') + if spec_junitxml is None: + junitxml_fp = _PotentialFile('results.xml') + else: + junitxml_fp = _mktemp_junitxml(spec_junitxml) + if spec_tracking is None: + tracking_fp = _PotentialFile('tracking') + else: + tracking_fp = _mktemp_tracking(spec_tracking) + return direction, junitxml_fp, tracking_fp + + +def _mktemp_junitxml(spec): + '''Test helper to generate a raw JUnit XML file. + + >>> fp = _mktemp_junitxml('00101') + >>> open(fp.name).read()[:11] + '' + + ''' + fp = tempfile.NamedTemporaryFile() + fp.write(b'') + + passed = '''\ + +''' + failed = '''\ + +Traceback (most recent call last): + File "/foo/bar/baz/buz.py", line 1, in <module> +NameError: name 'heck' is not defined + + +''' + + i = 0 + for c in spec: + if c == '0': + out = failed + elif c == '1': + out = passed + else: + raise ValueError(f'bad c: `{c}`') + fp.write(out.format(i=i).encode('utf8')) + i += 1 + + fp.write(b'') + fp.flush() + return fp + + +def _mktemp_tracking(spec): + '''Test helper to prefabricate a tracking file. + + >>> fp = _mktemp_tracking('01101') + >>> print(open(fp.name).read()[:-1]) + c1.t + c2.t + c4.t + + ''' + fp = tempfile.NamedTemporaryFile() + + i = 0 + for c in spec: + if c == '0': + pass + elif c == '1': + fp.write(f'c{i}.t\n'.encode('utf8')) + else: + raise ValueError(f'bad c: `{c}`') + i += 1 + + fp.flush() + return fp diff --git a/misc/python3/ratchet.sh b/misc/python3/ratchet.sh new file mode 100755 index 000000000..00acd60c3 --- /dev/null +++ b/misc/python3/ratchet.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +base="$(dirname $0)" +tracking="passing" + +# trial outputs some things that are only git-ignored in the root, so don't cd quite yet ... +set +e +trial --reporter subunitv2 allmydata | subunit2junitxml > "$base/results.xml" +set -e + +# Okay, now we're clear. +cd "$base" + +# Make sure ratchet.py itself is clean. +python3 -m doctest ratchet.py + +# Now see about Tahoe-LAFS ... +set +e +# P.S. Don't invoke as `python ratchet.py ...` because then Python swallows the +# exit code. +./ratchet.py up results.xml "$tracking" +code=$? +set -e + +# Emit a diff of the tracking file, to aid in the situation where changes are +# not discovered until CI (where TERM might `dumb`). +if [ $TERM = 'dumb' ]; then + export TERM=ansi +fi +git diff "$tracking" + +exit $code diff --git a/setup.py b/setup.py index f7f7b14f5..3a7d8ef8c 100644 --- a/setup.py +++ b/setup.py @@ -385,6 +385,7 @@ setup(name="tahoe-lafs", # also set in __init__.py "fixtures", "beautifulsoup4", "html5lib", + "junitxml", ] + tor_requires + i2p_requires, "tor": tor_requires, "i2p": i2p_requires, diff --git a/tox.ini b/tox.ini index af45f5da8..ab467abd0 100644 --- a/tox.ini +++ b/tox.ini @@ -48,6 +48,11 @@ commands = trial {env:TAHOE_LAFS_TRIAL_ARGS:--rterrors} {posargs:allmydata} tahoe --version +[testenv:py36] +# git inside of ratchet.sh needs $HOME. +passenv = HOME +commands = {toxinidir}/misc/python3/ratchet.sh + [testenv:integration] setenv = COVERAGE_PROCESS_START=.coveragerc From 8e8a215b83cdcc1a58a4a5086132b442530e8c23 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Fri, 3 Jul 2020 17:52:10 -0400 Subject: [PATCH 12/20] Is this it? towncrier just wants empty files? --- newsfragments/3325.other | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3325.other diff --git a/newsfragments/3325.other b/newsfragments/3325.other new file mode 100644 index 000000000..e69de29bb From e2d6b353e82ccbc95c869cf8bcfa436f5c089b1e Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Tue, 14 Jul 2020 06:42:45 -0400 Subject: [PATCH 13/20] Tighten up condition in depgraph.sh I ran into a circumstance where it appears that there were changes other than to the two files we care about, leading to an empty commit and a CI failure: https://app.circleci.com/pipelines/github/tahoe-lafs/tahoe-lafs/320/workflows/7d045f5f-1536-4cfa-b232-42837d4c9334/jobs/22127/steps --- misc/python3/depgraph.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/python3/depgraph.sh b/misc/python3/depgraph.sh index d5ad33bf7..a1ef21142 100755 --- a/misc/python3/depgraph.sh +++ b/misc/python3/depgraph.sh @@ -10,7 +10,7 @@ cd tahoe-depgraph # Generate the maybe-changed data. python "${TAHOE}"/misc/python3/tahoe-depgraph.py "${TAHOE}" -if git diff-index --quiet HEAD; then +if git diff-index --quiet HEAD tahoe-deps.json tahoe-ported.json; then echo "Declining to commit without any changes." exit 0 fi From 3f887f92799904c1003d0e2584f9b97048c58aaf Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Tue, 14 Jul 2020 20:11:58 -0400 Subject: [PATCH 14/20] Respond to review - Use the subunitv2-file reporter to allow for nicer error handling - Undo the depgraph.sh change, see if it really matters - Rename passing -> ratchet-passing to distinguish from ported modules tracker - Misc. documentation and ratchet.sh improvements --- .gitignore | 1 + misc/python3/depgraph.sh | 2 +- misc/python3/{passing => ratchet-passing} | 0 misc/python3/ratchet.py | 4 +--- misc/python3/ratchet.sh | 24 ++++++++++++++--------- setup.py | 1 + 6 files changed, 19 insertions(+), 13 deletions(-) rename misc/python3/{passing => ratchet-passing} (100%) diff --git a/.gitignore b/.gitignore index b33c1aab8..8191c173b 100644 --- a/.gitignore +++ b/.gitignore @@ -46,6 +46,7 @@ zope.interface-*.egg /.hypothesis/ /eliot.log /misc/python3/results.xml +/misc/python3/results.subunit2 # This is the plaintext of the private environment needed for some CircleCI # operations. It's never supposed to be checked in. diff --git a/misc/python3/depgraph.sh b/misc/python3/depgraph.sh index a1ef21142..d5ad33bf7 100755 --- a/misc/python3/depgraph.sh +++ b/misc/python3/depgraph.sh @@ -10,7 +10,7 @@ cd tahoe-depgraph # Generate the maybe-changed data. python "${TAHOE}"/misc/python3/tahoe-depgraph.py "${TAHOE}" -if git diff-index --quiet HEAD tahoe-deps.json tahoe-ported.json; then +if git diff-index --quiet HEAD; then echo "Declining to commit without any changes." exit 0 fi diff --git a/misc/python3/passing b/misc/python3/ratchet-passing similarity index 100% rename from misc/python3/passing rename to misc/python3/ratchet-passing diff --git a/misc/python3/ratchet.py b/misc/python3/ratchet.py index 136fdcf4c..cb672cf67 100755 --- a/misc/python3/ratchet.py +++ b/misc/python3/ratchet.py @@ -26,9 +26,6 @@ The exit codes are: 1 - changes observed, whether welcome or unwelcome 2 - invocation error -Be sure to call as `./ratchet.py` and not `python ratchet.py` if you want to -see our exit code instead of Python's. - If does not exist, you'll get a FileNotFoundError: >>> _test('up', None, None) # doctest: +ELLIPSIS @@ -231,6 +228,7 @@ def main(direction, junitxml_path, tracking_path): results = JUnitXMLFile(junitxml_path).parse() if tracking_path == '...': + # Shortcut to aid in debugging XML parsing issues. results.report() return diff --git a/misc/python3/ratchet.sh b/misc/python3/ratchet.sh index 00acd60c3..a6a2c53c3 100755 --- a/misc/python3/ratchet.sh +++ b/misc/python3/ratchet.sh @@ -1,10 +1,18 @@ #!/usr/bin/env bash -base="$(dirname $0)" -tracking="passing" +set -euxo pipefail +tracking_filename="ratchet-passing" -# trial outputs some things that are only git-ignored in the root, so don't cd quite yet ... +# Start somewhere predictable. +cd "$(dirname $0)" +base=$(pwd) + +# Actually, though, trial outputs some things that are only gitignored in the project root. +cd "../.." + +# Since both of the next calls are expected to exit non-0, relax our guard. set +e -trial --reporter subunitv2 allmydata | subunit2junitxml > "$base/results.xml" +SUBUNITREPORTER_OUTPUT_PATH="$base/results.subunit2" trial --reporter subunitv2-file allmydata +subunit2junitxml < "$base/results.subunit2" > "$base/results.xml" set -e # Okay, now we're clear. @@ -13,11 +21,9 @@ cd "$base" # Make sure ratchet.py itself is clean. python3 -m doctest ratchet.py -# Now see about Tahoe-LAFS ... +# Now see about Tahoe-LAFS (also expected to fail) ... set +e -# P.S. Don't invoke as `python ratchet.py ...` because then Python swallows the -# exit code. -./ratchet.py up results.xml "$tracking" +python3 ratchet.py up results.xml "$tracking_filename" code=$? set -e @@ -26,6 +32,6 @@ set -e if [ $TERM = 'dumb' ]; then export TERM=ansi fi -git diff "$tracking" +git diff "$tracking_filename" exit $code diff --git a/setup.py b/setup.py index 3a7d8ef8c..f22ca0f4c 100644 --- a/setup.py +++ b/setup.py @@ -142,6 +142,7 @@ tor_requires = [ i2p_requires = [ # txi2p has Python 3 support, but it's unreleased: https://github.com/str4d/txi2p/issues/10. + # URL lookups are in PEP-508 (via https://stackoverflow.com/a/54794506). # Also see the comment in tor_requires. "txi2p @ git+https://github.com/str4d/txi2p@0611b9a86172cb70d2f5e415a88eee9f230590b3#egg=txi2p", ] From ed6e1cb7ef948a41aa99cfbb167be3bfca3c5f7e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 15 Jul 2020 15:51:09 -0400 Subject: [PATCH 15/20] Fix formatting. --- src/allmydata/util/base32.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/base32.py b/src/allmydata/util/base32.py index 1d1f3ce4b..3d5b1fc8f 100644 --- a/src/allmydata/util/base32.py +++ b/src/allmydata/util/base32.py @@ -55,7 +55,7 @@ BASE32STR_1byte = BASE32CHAR+BASE32CHAR_3bits BASE32STR_2bytes = BASE32CHAR+b'{3}'+BASE32CHAR_1bits BASE32STR_3bytes = BASE32CHAR+b'{4}'+BASE32CHAR_4bits BASE32STR_4bytes = BASE32CHAR+b'{6}'+BASE32CHAR_2bits -BASE32STR_anybytes =b'((?:%s{8})*' % (BASE32CHAR,) + b"(?:|%s|%s|%s|%s))" % (BASE32STR_1byte, BASE32STR_2bytes, BASE32STR_3bytes, BASE32STR_4bytes) +BASE32STR_anybytes = bytes(b'((?:%s{8})*') % (BASE32CHAR,) + bytes(b"(?:|%s|%s|%s|%s))") % (BASE32STR_1byte, BASE32STR_2bytes, BASE32STR_3bytes, BASE32STR_4bytes) def b2a(os): """ From 6af32fdfa7bbb356f38855281c1ea653a54d930d Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 15 Jul 2020 15:59:46 -0400 Subject: [PATCH 16/20] Make sure the public API exposes native bytes, not Future bytes. --- src/allmydata/util/base32.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/allmydata/util/base32.py b/src/allmydata/util/base32.py index 3d5b1fc8f..f47960b87 100644 --- a/src/allmydata/util/base32.py +++ b/src/allmydata/util/base32.py @@ -12,6 +12,17 @@ from future.utils import PY2 if PY2: from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, int, list, object, range, str, max, min # noqa: F401 +if PY2: + def backwardscompat_bytes(b): + """ + Replace Future bytes with native Python 2 bytes, so % works + consistently until other modules are ported. + """ + return getattr(b, "__native__", lambda: b)() +else: + def backwardscompat_bytes(b): + return b + import base64 from allmydata.util.assertutil import precondition @@ -46,16 +57,16 @@ def get_trailing_chars_without_lsbs(N): d = {} return b''.join(_get_trailing_chars_without_lsbs(N, d=d)) -BASE32CHAR =b'['+get_trailing_chars_without_lsbs(0)+b']' -BASE32CHAR_4bits =b'['+get_trailing_chars_without_lsbs(1)+b']' -BASE32CHAR_3bits =b'['+get_trailing_chars_without_lsbs(2)+b']' -BASE32CHAR_2bits =b'['+get_trailing_chars_without_lsbs(3)+b']' -BASE32CHAR_1bits =b'['+get_trailing_chars_without_lsbs(4)+b']' -BASE32STR_1byte = BASE32CHAR+BASE32CHAR_3bits -BASE32STR_2bytes = BASE32CHAR+b'{3}'+BASE32CHAR_1bits -BASE32STR_3bytes = BASE32CHAR+b'{4}'+BASE32CHAR_4bits -BASE32STR_4bytes = BASE32CHAR+b'{6}'+BASE32CHAR_2bits -BASE32STR_anybytes = bytes(b'((?:%s{8})*') % (BASE32CHAR,) + bytes(b"(?:|%s|%s|%s|%s))") % (BASE32STR_1byte, BASE32STR_2bytes, BASE32STR_3bytes, BASE32STR_4bytes) +BASE32CHAR = backwardscompat_bytes(b'['+get_trailing_chars_without_lsbs(0)+b']') +BASE32CHAR_4bits = backwardscompat_bytes(b'['+get_trailing_chars_without_lsbs(1)+b']') +BASE32CHAR_3bits = backwardscompat_bytes(b'['+get_trailing_chars_without_lsbs(2)+b']') +BASE32CHAR_2bits = backwardscompat_bytes(b'['+get_trailing_chars_without_lsbs(3)+b']') +BASE32CHAR_1bits = backwardscompat_bytes(b'['+get_trailing_chars_without_lsbs(4)+b']') +BASE32STR_1byte = backwardscompat_bytes(BASE32CHAR+BASE32CHAR_3bits) +BASE32STR_2bytes = backwardscompat_bytes(BASE32CHAR+b'{3}'+BASE32CHAR_1bits) +BASE32STR_3bytes = backwardscompat_bytes(BASE32CHAR+b'{4}'+BASE32CHAR_4bits) +BASE32STR_4bytes = backwardscompat_bytes(BASE32CHAR+b'{6}'+BASE32CHAR_2bits) +BASE32STR_anybytes = backwardscompat_bytes(bytes(b'((?:%s{8})*') % (BASE32CHAR,) + bytes(b"(?:|%s|%s|%s|%s))") % (BASE32STR_1byte, BASE32STR_2bytes, BASE32STR_3bytes, BASE32STR_4bytes)) def b2a(os): """ From efbae9b3e36777e0bab7dbd1fc53e19a9162603a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 16 Jul 2020 14:33:53 -0400 Subject: [PATCH 17/20] Hard code some known values, generated on the master branch preceding these changes. --- src/allmydata/test/test_base62.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_base62.py b/src/allmydata/test/test_base62.py index 6f3ef8293..482a968ea 100644 --- a/src/allmydata/test/test_base62.py +++ b/src/allmydata/test/test_base62.py @@ -27,7 +27,7 @@ from allmydata.util import base62, mathutil def insecurerandstr(n): return bytes(list(map(random.randrange, [0]*n, [256]*n))) -class T(unittest.TestCase): +class Base62(unittest.TestCase): def _test_num_octets_that_encode_to_this_many_chars(self, chars, octets): assert base62.num_octets_that_encode_to_this_many_chars(chars) == octets, "%s != %s <- %s" % (octets, base62.num_octets_that_encode_to_this_many_chars(chars), chars) @@ -43,6 +43,22 @@ class T(unittest.TestCase): def test_roundtrip(self, input_bytes): self._test_roundtrip(input_bytes) + def test_known_values(self): + """Known values to ensure the algorithm hasn't changed.""" + + def check_expected(plaintext, encoded): + result1 = base62.b2a(plaintext) + self.assertEqual(encoded, result1) + result2 = base62.a2b(encoded) + self.assertEqual(plaintext, result2) + + check_expected(b"hello", b'7tQLFHz') + check_expected(b"", b'0') + check_expected(b"zzz", b'0Xg7e') + check_expected(b"\x36\xffWAT", b'49pq4mq') + check_expected(b"1234 22323", b'1A0afZe9mxSZpz') + check_expected(b"______", b'0TmAuCHJX') + def test_num_octets_that_encode_to_this_many_chars(self): return self._test_num_octets_that_encode_to_this_many_chars(2, 1) return self._test_num_octets_that_encode_to_this_many_chars(3, 2) From 58c4908d46aff07352f875e351ccb07499dc76db Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 16 Jul 2020 14:36:49 -0400 Subject: [PATCH 18/20] More tests are passing. --- misc/python3/ratchet-passing | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/misc/python3/ratchet-passing b/misc/python3/ratchet-passing index e659c95b6..a6e1de68b 100644 --- a/misc/python3/ratchet-passing +++ b/misc/python3/ratchet-passing @@ -1,4 +1,19 @@ allmydata.test.mutable.test_exceptions.Exceptions.test_repr +allmydata.test.test_base32.Base32.test_a2b +allmydata.test.test_base32.Base32.test_a2b_b2a_match_Pythons +allmydata.test.test_base32.Base32.test_b2a +allmydata.test.test_base32.Base32.test_b2a_or_none +allmydata.test.test_base62.Base62.test_ende_0x00 +allmydata.test.test_base62.Base62.test_ende_0x000000 +allmydata.test.test_base62.Base62.test_ende_0x01 +allmydata.test.test_base62.Base62.test_ende_0x0100 +allmydata.test.test_base62.Base62.test_ende_0x010000 +allmydata.test.test_base62.Base62.test_ende_longrandstr +allmydata.test.test_base62.Base62.test_ende_randstr +allmydata.test.test_base62.Base62.test_known_values +allmydata.test.test_base62.Base62.test_num_octets_that_encode_to_this_many_chars +allmydata.test.test_base62.Base62.test_odd_sizes +allmydata.test.test_base62.Base62.test_roundtrip allmydata.test.test_deferredutil.DeferredUtilTests.test_failure allmydata.test.test_deferredutil.DeferredUtilTests.test_gather_results allmydata.test.test_deferredutil.DeferredUtilTests.test_success From 2f693c47f9597f3b7071c7370c44a2850bce5a60 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 16 Jul 2020 14:37:26 -0400 Subject: [PATCH 19/20] Don't leak variable on Python 2. --- src/allmydata/util/base32.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/allmydata/util/base32.py b/src/allmydata/util/base32.py index f47960b87..cb21d1a8a 100644 --- a/src/allmydata/util/base32.py +++ b/src/allmydata/util/base32.py @@ -94,7 +94,9 @@ NUM_OS_TO_NUM_QS=(0, 2, 4, 5, 7,) NUM_QS_TO_NUM_OS=(0, 1, 1, 2, 2, 3, 3, 4) NUM_QS_LEGIT=(1, 0, 1, 0, 1, 1, 0, 1,) -NUM_QS_TO_NUM_BITS=tuple([x*8 for x in NUM_QS_TO_NUM_OS]) +NUM_QS_TO_NUM_BITS=tuple([_x*8 for _x in NUM_QS_TO_NUM_OS]) +if PY2: + del _x # A fast way to determine whether a given string *could* be base-32 encoded data, assuming that the # original data had 8K bits for a positive integer K. From 8d143af43e7855bec8c3e057339d9e377c77c034 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 16 Jul 2020 14:40:30 -0400 Subject: [PATCH 20/20] Another assertion. --- src/allmydata/test/test_base62.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/test/test_base62.py b/src/allmydata/test/test_base62.py index 482a968ea..d1d0c72f3 100644 --- a/src/allmydata/test/test_base62.py +++ b/src/allmydata/test/test_base62.py @@ -38,6 +38,8 @@ class Base62(unittest.TestCase): self.assertIsInstance(encoded, bytes) self.assertIsInstance(bs, bytes) self.assertIsInstance(decoded, bytes) + # Encoded string only uses values from the base62 allowed characters: + self.assertFalse(set(encoded) - set(base62.chars)) @given(input_bytes=st.binary(max_size=100)) def test_roundtrip(self, input_bytes):