URI: 
       tMerge pull request #736 from chrisglass/more-tests-2 - electrum - Electrum Bitcoin wallet
  HTML git clone https://git.parazyd.org/electrum
   DIR Log
   DIR Files
   DIR Refs
   DIR Submodules
       ---
   DIR commit 7338ac3c5417d037b5c7a4fed4f66bdc20a322e9
   DIR parent 029e0b8b0d788463feb56cb34540d9dcc9f31e93
  HTML Author: ThomasV <thomasv1@gmx.de>
       Date:   Thu, 26 Jun 2014 11:58:25 +0200
       
       Merge pull request #736 from chrisglass/more-tests-2
       
       Add tests to the SimpleConfig object (resubmit)
       Diffstat:
         M .travis.yml                         |       2 +-
         M electrum                            |       7 +++----
         M lib/blockchain.py                   |       4 +++-
         M lib/daemon.py                       |       4 +++-
         M lib/network.py                      |       4 +++-
         M lib/simple_config.py                |     236 +++++++++++++++----------------
         A lib/tests/test_simple_config.py     |     222 ++++++++++++++++++++++++++++++
       
       7 files changed, 351 insertions(+), 128 deletions(-)
       ---
   DIR diff --git a/.travis.yml b/.travis.yml
       t@@ -2,4 +2,4 @@ language: python
        python:
            - "2.7"
        install: "pip install slowaes==0.1a1 ecdsa>=0.9 pbkdf2 requests pyasn1 pyasn1-modules tlslite>=0.4.5 qrcode"
       -script: nosetests lib
       +script: nosetests -e gui
   DIR diff --git a/electrum b/electrum
       t@@ -103,16 +103,15 @@ def print_help_cb(self, opt, value, parser):
            print_help(parser)
        
        
       -def run_command(cmd, password=None, args=[]):
       -    import socket
       +def run_command(cmd, password=None, args=None):
       +    if args is None:
       +        args = []  # Do not use mutables as default values!
            if cmd.requires_network and not options.offline:
                network = NetworkProxy(config)
                if not network.start(start_daemon= (True if cmd.name!='daemon' else False)):
                    print "Daemon not running"
                    sys.exit(1)
        
       -
       -
                if wallet:
                    wallet.start_threads(network)
                    wallet.update()
   DIR diff --git a/lib/blockchain.py b/lib/blockchain.py
       t@@ -241,7 +241,9 @@ class Blockchain(threading.Thread):
                        return h 
        
        
       -    def get_target(self, index, chain=[]):
       +    def get_target(self, index, chain=None):
       +        if chain is None:
       +            chain = []  # Do not use mutables as default values!
        
                max_target = 0x00000000FFFF0000000000000000000000000000000000000000000000000000
                if index == 0: return 0x1d00ffff, max_target
   DIR diff --git a/lib/daemon.py b/lib/daemon.py
       t@@ -34,7 +34,9 @@ class NetworkProxy(threading.Thread):
            # connects to daemon
            # sends requests, runs callbacks
        
       -    def __init__(self, config = {}):
       +    def __init__(self, config=None):
       +        if config is None:
       +            config = {}  # Do not use mutables as default arguments!
                threading.Thread.__init__(self)
                self.daemon = True
                self.config = SimpleConfig(config) if type(config) == type({}) else config
   DIR diff --git a/lib/network.py b/lib/network.py
       t@@ -72,7 +72,9 @@ from simple_config import SimpleConfig
        
        class Network(threading.Thread):
        
       -    def __init__(self, config = {}):
       +    def __init__(self, config=None):
       +        if config is None:
       +            config = {}  # Do not use mutables as default values!
                threading.Thread.__init__(self)
                self.daemon = True
                self.config = SimpleConfig(config) if type(config) == type({}) else config
   DIR diff --git a/lib/simple_config.py b/lib/simple_config.py
       t@@ -1,62 +1,94 @@
       -import json
        import ast
        import threading
        import os
        
        from util import user_dir, print_error, print_msg
        
       +SYSTEM_CONFIG_PATH = "/etc/electrum.conf"
        
        config = None
       +
       +
        def get_config():
            global config
            return config
        
       +
        def set_config(c):
            global config
            config = c
        
        
       -class SimpleConfig:
       +class SimpleConfig(object):
            """
       -The SimpleConfig class is responsible for handling operations involving
       -configuration files.  The constructor reads and stores the system and 
       -user configurations from electrum.conf into separate dictionaries within
       -a SimpleConfig instance then reads the wallet file.
       -"""
       -    def __init__(self, options={}):
       -        self.lock = threading.Lock()
       -
       -        # system conf, readonly
       -        self.system_config = {}
       +    The SimpleConfig class is responsible for handling operations involving
       +    configuration files.
       +
       +    There are 3 different sources of possible configuration values:
       +        1. Command line options.
       +        2. User configuration (in the user's config directory)
       +        3. System configuration (in /etc/)
       +    They are taken in order (1. overrides config options set in 2., that
       +    override config set in 3.)
       +    """
       +    def __init__(self, options=None, read_system_config_function=None,
       +                 read_user_config_function=None, read_user_dir_function=None):
       +
       +        # This is the holder of actual options for the current user.
       +        self.current_options = {}
       +        # This lock needs to be acquired for updating and reading the config in
       +        # a thread-safe way.
       +        self.lock = threading.RLock()
       +        # The path for the config directory. This is set later by init_path()
       +        self.path = None
       +
       +        if options is None:
       +            options = {}  # Having a mutable as a default value is a bad idea.
       +
       +        # The following two functions are there for dependency injection when
       +        # testing.
       +        if read_system_config_function is None:
       +            read_system_config_function = read_system_config
       +        if read_user_config_function is None:
       +            read_user_config_function = read_user_config
       +        if read_user_dir_function is None:
       +            self.user_dir = user_dir
       +        else:
       +            self.user_dir = read_user_dir_function
       +
       +        # Save the command-line keys to make sure we don't override them.
       +        self.command_line_keys = options.keys()
       +        # Save the system config keys to make sure we don't override them.
       +        self.system_config_keys = []
       +
                if options.get('portable') is not True:
       -            self.read_system_config()
       +            # system conf
       +            system_config = read_system_config_function()
       +            self.system_config_keys = system_config.keys()
       +            self.current_options.update(system_config)
        
       -        # command-line options
       -        self.options_config = options
       +        # update the current options with the command line options last (to
       +        # override both others).
       +        self.current_options.update(options)
        
                # init path
                self.init_path()
        
       -        # user conf, writeable
       -        self.user_config = {}
       -        self.read_user_config()
       -
       -        set_config(self)
       -
       +        # user config.
       +        self.user_config = read_user_config_function(self.path)
       +        # The user config is overwritten by the current config!
       +        self.user_config.update(self.current_options)
       +        self.current_options = self.user_config
        
       +        set_config(self)  # Make a singleton instance of 'self'
        
            def init_path(self):
       -
                # Read electrum path in the command line configuration
       -        self.path = self.options_config.get('electrum_path')
       -
       -        # Read electrum path in the system configuration
       -        if self.path is None:
       -            self.path = self.system_config.get('electrum_path')
       +        self.path = self.current_options.get('electrum_path')
        
                # If not set, use the user's default data directory.
                if self.path is None:
       -            self.path = user_dir()
       +            self.path = self.user_dir()
        
                # Make directory if it does not yet exist.
                if not os.path.exists(self.path):
       t@@ -64,110 +96,32 @@ a SimpleConfig instance then reads the wallet file.
        
                print_error( "electrum directory", self.path)
        
       -        # portable wallet: use the same directory for wallet and headers file
       -        #if options.get('portable'):
       -        #    self.wallet_config['blockchain_headers_path'] = os.path.dirname(self.path)
       -            
            def set_key(self, key, value, save = True):
       -        # find where a setting comes from and save it there
       -        if self.options_config.get(key) is not None:
       -            print "Warning: not changing '%s' because it was passed as a command-line option"%key
       +        if not self.is_modifiable(key):
       +            print "Warning: not changing key '%s' because it is not modifiable" \
       +                  " (passed as command line option or defined in /etc/electrum.conf)"%key
                    return
        
       -        elif self.system_config.get(key) is not None:
       -            if str(self.system_config[key]) != str(value):
       -                print "Warning: not changing '%s' because it was set in the system configuration"%key
       -
       -        else:
       -
       -            with self.lock:
       -                self.user_config[key] = value
       -                if save: 
       -                    self.save_user_config()
       -
       +        with self.lock:
       +            self.user_config[key] = value
       +            self.current_options[key] = value
       +            if save:
       +                self.save_user_config()
        
       +        return
        
            def get(self, key, default=None):
       -
                out = None
       -
       -        # 1. command-line options always override everything
       -        if self.options_config.has_key(key) and self.options_config.get(key) is not None:
       -            out = self.options_config.get(key)
       -
       -        # 2. user configuration 
       -        elif self.user_config.has_key(key):
       -            out = self.user_config.get(key)
       -
       -        # 2. system configuration
       -        elif self.system_config.has_key(key):
       -            out = self.system_config.get(key)
       -
       -        if out is None and default is not None:
       -            out = default
       -
       -        # try to fix the type
       -        if default is not None and type(out) != type(default):
       -            import ast
       -            try:
       -                out = ast.literal_eval(out)
       -            except Exception:
       -                print "type error for '%s': using default value"%key
       -                out = default
       -
       +        with self.lock:
       +            out = self.current_options.get(key, default)
                return out
        
       -
            def is_modifiable(self, key):
       -        """Check if the config file is modifiable."""
       -        if self.options_config.has_key(key):
       +        if key in self.command_line_keys:
                    return False
       -        elif self.user_config.has_key(key):
       -            return True
       -        elif self.system_config.has_key(key):
       +        if key in self.system_config_keys:
                    return False
       -        else:
       -            return True
       -
       -
       -    def read_system_config(self):
       -        """Parse and store the system config settings in electrum.conf into system_config[]."""
       -        name = '/etc/electrum.conf'
       -        if os.path.exists(name):
       -            try:
       -                import ConfigParser
       -            except ImportError:
       -                print "cannot parse electrum.conf. please install ConfigParser"
       -                return
       -                
       -            p = ConfigParser.ConfigParser()
       -            p.read(name)
       -            try:
       -                for k, v in p.items('client'):
       -                    self.system_config[k] = v
       -            except ConfigParser.NoSectionError:
       -                pass
       -
       -
       -    def read_user_config(self):
       -        """Parse and store the user config settings in electrum.conf into user_config[]."""
       -        if not self.path: return
       -
       -        path = os.path.join(self.path, "config")
       -        if os.path.exists(path):
       -            try:
       -                with open(path, "r") as f:
       -                    data = f.read()
       -            except IOError:
       -                return
       -            try:
       -                d = ast.literal_eval( data )  #parse raw data from reading wallet file
       -            except Exception:
       -                print_msg("Error: Cannot read config file.")
       -                return
       -
       -            self.user_config = d
       -
       +        return True
        
            def save_user_config(self):
                if not self.path: return
       t@@ -180,3 +134,45 @@ a SimpleConfig instance then reads the wallet file.
                if self.get('gui') != 'android':
                    import stat
                    os.chmod(path, stat.S_IREAD | stat.S_IWRITE)
       +
       +def read_system_config(path=SYSTEM_CONFIG_PATH):
       +    """Parse and return the system config settings in /etc/electrum.conf."""
       +    result = {}
       +    if os.path.exists(path):
       +        try:
       +            import ConfigParser
       +        except ImportError:
       +            print "cannot parse electrum.conf. please install ConfigParser"
       +            return
       +
       +        p = ConfigParser.ConfigParser()
       +        try:
       +            p.read(path)
       +            for k, v in p.items('client'):
       +                result[k] = v
       +        except (ConfigParser.NoSectionError, ConfigParser.MissingSectionHeaderError):
       +            pass
       +
       +    return result
       +
       +def read_user_config(path):
       +    """Parse and store the user config settings in electrum.conf into user_config[]."""
       +    if not path: return {}  # Return a dict, since we will call update() on it.
       +
       +    config_path = os.path.join(path, "config")
       +    result = {}
       +    if os.path.exists(config_path):
       +        try:
       +
       +            with open(config_path, "r") as f:
       +                data = f.read()
       +            result = ast.literal_eval( data )  #parse raw data from reading wallet file
       +
       +        except Exception:
       +            print_msg("Error: Cannot read config file.")
       +            result = {}
       +
       +        if not type(result) is dict:
       +            return {}
       +
       +    return result
   DIR diff --git a/lib/tests/test_simple_config.py b/lib/tests/test_simple_config.py
       t@@ -0,0 +1,222 @@
       +import sys
       +import os
       +import unittest
       +import tempfile
       +import shutil
       +
       +from StringIO import StringIO
       +from lib.simple_config import (SimpleConfig, read_system_config,
       +                               read_user_config)
       +
       +
       +class Test_SimpleConfig(unittest.TestCase):
       +
       +    def setUp(self):
       +        super(Test_SimpleConfig, self).setUp()
       +        # make sure "read_user_config" and "user_dir" return a temporary directory.
       +        self.electrum_dir = tempfile.mkdtemp()
       +        # Do the same for the user dir to avoid overwriting the real configuration
       +        # for development machines with electrum installed :)
       +        self.user_dir = tempfile.mkdtemp()
       +
       +        self.options = {"electrum_path": self.electrum_dir}
       +        self._saved_stdout = sys.stdout
       +        self._stdout_buffer = StringIO()
       +        sys.stdout = self._stdout_buffer
       +
       +    def tearDown(self):
       +        super(Test_SimpleConfig, self).tearDown()
       +        # Remove the temporary directory after each test (to make sure we don't
       +        # pollute /tmp for nothing.
       +        shutil.rmtree(self.electrum_dir)
       +        shutil.rmtree(self.user_dir)
       +
       +        # Restore the "real" stdout
       +        sys.stdout = self._saved_stdout
       +
       +    def test_simple_config_command_line_overrides_everything(self):
       +        """Options passed by command line override all other configuration
       +        sources"""
       +        fake_read_system = lambda : {"electrum_path": "a"}
       +        fake_read_user = lambda _: {"electrum_path": "b"}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options=self.options,
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        self.assertEqual(self.options.get("electrum_path"),
       +                         config.get("electrum_path"))
       +
       +    def test_simple_config_system_config_overrides_user_config(self):
       +        """Options passed in system config override user config."""
       +        fake_read_system = lambda : {"electrum_path": self.electrum_dir}
       +        fake_read_user = lambda _: {"electrum_path": "b"}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options=None,
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        self.assertEqual(self.options.get("electrum_path"),
       +                         config.get("electrum_path"))
       +
       +    def test_simple_config_system_config_ignored_if_portable(self):
       +        """If electrum is started with the "portable" flag, system
       +        configuration is completely ignored."""
       +        another_path = tempfile.mkdtemp()
       +        fake_read_system = lambda : {"electrum_path": self.electrum_dir}
       +        fake_read_user = lambda _: {"electrum_path": another_path}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options={"portable": True},
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        self.assertEqual(another_path, config.get("electrum_path"))
       +
       +    def test_simple_config_user_config_is_used_if_others_arent_specified(self):
       +        """If no system-wide configuration and no command-line options are
       +        specified, the user configuration is used instead."""
       +        fake_read_system = lambda : {}
       +        fake_read_user = lambda _: {"electrum_path": self.electrum_dir}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options=None,
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        self.assertEqual(self.options.get("electrum_path"),
       +                         config.get("electrum_path"))
       +
       +    def test_cannot_set_options_passed_by_command_line(self):
       +        fake_read_system = lambda : {}
       +        fake_read_user = lambda _: {"electrum_path": "b"}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options=self.options,
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        config.set_key("electrum_path", "c")
       +        self.assertEqual(self.options.get("electrum_path"),
       +                         config.get("electrum_path"))
       +
       +    def test_cannot_set_options_from_system_config(self):
       +        fake_read_system = lambda : {"electrum_path": self.electrum_dir}
       +        fake_read_user = lambda _: {}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options={},
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        config.set_key("electrum_path", "c")
       +        self.assertEqual(self.options.get("electrum_path"),
       +                         config.get("electrum_path"))
       +
       +    def test_can_set_options_set_in_user_config(self):
       +        another_path = tempfile.mkdtemp()
       +        fake_read_system = lambda : {}
       +        fake_read_user = lambda _: {"electrum_path": self.electrum_dir}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options={},
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        config.set_key("electrum_path", another_path)
       +        self.assertEqual(another_path, config.get("electrum_path"))
       +
       +    def test_can_set_options_from_system_config_if_portable(self):
       +        """If the "portable" flag is set, the user can overwrite system
       +        configuration options."""
       +        another_path = tempfile.mkdtemp()
       +        fake_read_system = lambda : {"electrum_path": self.electrum_dir}
       +        fake_read_user = lambda _: {}
       +        read_user_dir = lambda : self.user_dir
       +        config = SimpleConfig(options={"portable": True},
       +                              read_system_config_function=fake_read_system,
       +                              read_user_config_function=fake_read_user,
       +                              read_user_dir_function=read_user_dir)
       +        config.set_key("electrum_path", another_path)
       +        self.assertEqual(another_path, config.get("electrum_path"))
       +
       +
       +class TestSystemConfig(unittest.TestCase):
       +
       +    sample_conf = """
       +[client]
       +gap_limit = 5
       +
       +[something_else]
       +everything = 42
       +"""
       +
       +    def setUp(self):
       +        super(TestSystemConfig, self).setUp()
       +        self.thefile = tempfile.mkstemp(suffix=".electrum.test.conf")[1]
       +
       +    def tearDown(self):
       +        super(TestSystemConfig, self).tearDown()
       +        os.remove(self.thefile)
       +
       +    def test_read_system_config_file_does_not_exist(self):
       +        somefile = "/foo/I/do/not/exist/electrum.conf"
       +        result = read_system_config(somefile)
       +        self.assertEqual({}, result)
       +
       +    def test_read_system_config_file_returns_file_options(self):
       +        with open(self.thefile, "w") as f:
       +            f.write(self.sample_conf)
       +
       +        result = read_system_config(self.thefile)
       +        self.assertEqual({"gap_limit": "5"}, result)
       +
       +    def test_read_system_config_file_no_sections(self):
       +
       +        with open(self.thefile, "w") as f:
       +            f.write("gap_limit = 5")  # The file has no sections at all
       +
       +        result = read_system_config(self.thefile)
       +        self.assertEqual({}, result)
       +
       +
       +class TestUserConfig(unittest.TestCase):
       +
       +    def setUp(self):
       +        super(TestUserConfig, self).setUp()
       +        self._saved_stdout = sys.stdout
       +        self._stdout_buffer = StringIO()
       +        sys.stdout = self._stdout_buffer
       +
       +        self.user_dir = tempfile.mkdtemp()
       +
       +    def tearDown(self):
       +        super(TestUserConfig, self).tearDown()
       +        shutil.rmtree(self.user_dir)
       +        sys.stdout = self._saved_stdout
       +
       +    def test_no_path_means_no_result(self):
       +       result = read_user_config(None)
       +       self.assertEqual({}, result)
       +
       +    def test_path_with_reprd_dict(self):
       +        thefile = os.path.join(self.user_dir, "config")
       +        payload = {"gap_limit": 5}
       +        with open(thefile, "w") as f:
       +            f.write(repr(payload))
       +
       +        result = read_user_config(self.user_dir)
       +        self.assertEqual(payload, result)
       +
       +    def test_path_without_config_file(self):
       +        """We pass a path but if does not contain a "config" file."""
       +        result = read_user_config(self.user_dir)
       +        self.assertEqual({}, result)
       +
       +    def test_path_with_reprd_object(self):
       +
       +        class something(object):
       +            pass
       +
       +        thefile = os.path.join(self.user_dir, "config")
       +        payload = something()
       +        with open(thefile, "w") as f:
       +            f.write(repr(payload))
       +
       +        result = read_user_config(self.user_dir)
       +        self.assertEqual({}, result)