Merge pull request #4 from erickwilder/stop-discover-if-valid-config-…

…files-were-found

Stop discover if valid config files were found

CHANGES.txt

@@ -1,3 +1,12 @@
+0.3.2
+=====
+
+  - Stop directories from being traversed up when valid configurations were found.
+  - Validates invalid unicode data on INI files (and skip them)
+  - Better Python3 support with use of ConfigParser.read_file
+  - Code cleanup
+  - More test cases for ConfigurationDiscovery added
+
 0.3.1
 =====
 

prettyconf/configuration.py

@@ -44,7 +44,7 @@ def _discover(self):
 
             self._config_files += self._scan_path(path)
 
-            if path == os.path.sep:
+            if self._config_files or path == os.path.sep:
                 break
 
             path = os.path.dirname(path)

prettyconf/loaders.py

@@ -1,9 +1,8 @@
 # coding: utf-8
 
-
+import sys
 import os
 from glob import glob
-from shlex import shlex
 
 try:
     from ConfigParser import SafeConfigParser as ConfigParser, NoOptionError
@@ -155,7 +154,14 @@ def __init__(self, filename, section=None):
         self.parser = ConfigParser(allow_no_value=True)
 
         with open(self.filename) as inifile:
-            self.parser.readfp(inifile)
+            try:
+                if sys.version_info[0] < 3:
+                    # ConfigParser.readfp is deprecated for Python3, read_file replaces it
+                    self.parser.readfp(inifile)
+                else:
+                    self.parser.read_file(inifile)
+            except UnicodeDecodeError:
+                raise InvalidConfigurationFile()
 
         if not self.parser.has_section(self.section):
             raise InvalidConfigurationFile("Missing [{}] section in {}".format(self.section, self.filename))

tests/files/invalid_chars.cfg

@@ -0,0 +1 @@
+볲捫寈婞�窍륮ஓ�茰줤��⩟趇廗～ᾧ락踃暂�

tests/test_cfgfile.py

@@ -12,7 +12,7 @@ def setUp(self):
 
     def test_fail_invalid_settings_file(self):
         with self.assertRaises(InvalidConfigurationFile):
-            IniFileConfigurationLoader(self.test_files_path + "/invalid.ini")
+            IniFileConfigurationLoader(self.test_files_path + "/invalid_section.ini")
 
     def test_config_file_parsing(self):
         config = IniFileConfigurationLoader(self.inifile)
@@ -38,6 +38,6 @@ def test_config_file_parsing(self):
 
     def test_list_config_filenames(self):
         filenames = IniFileConfigurationLoader.get_filenames(self.test_files_path)
-        self.assertEqual(len(filenames), 2)
+        self.assertEqual(len(filenames), 3)
         self.assertIn(self.test_files_path + "/config.ini", filenames)
-        self.assertIn(self.test_files_path + "/invalid.ini", filenames)
+        self.assertIn(self.test_files_path + "/invalid_section.ini", filenames)

tests/test_filediscover.py

@@ -11,20 +11,45 @@
 class ConfigFilesDiscoveryTestCase(BaseTestCase):
     def setUp(self):
         super(ConfigFilesDiscoveryTestCase, self).setUp()
+
+    def test_config_file_parsing(self):
         self._create_file(self.test_files_path + "/../.env")
         self._create_file(self.test_files_path + "/../setup.cfg")  # invalid settings
         self._create_file(self.test_files_path + "/../settings.ini", "[settings]")
-        self._create_file(self.test_files_path + "/../../.env")
-
-    def test_config_file_parsing(self):
         discovery = ConfigurationDiscovery(os.path.dirname(self.test_files_path))
-        self.assertEqual(len(discovery.config_files), 3)  # 3 *valid* files created at .setUp()
+        self.assertEqual(len(discovery.config_files), 2)  # 2 *valid* files created
 
     def test_abort_discovery_from_invalid_path(self):
+        self._create_file(self.test_files_path + '/.env')
         with self.assertRaises(InvalidPath):
             # filename is not a valid starting path...
-            _ = ConfigurationDiscovery(self.test_files_path + "/.env").config_files
+            ConfigurationDiscovery(self.test_files_path + "/.env").config_files
 
+    def test_abort_discovery_with_non_existing_path(self):
         with self.assertRaises(InvalidPath):
             # ... and missing path either
-            _ = ConfigurationDiscovery(self.test_files_path + "/missing").config_files
+            ConfigurationDiscovery(self.test_files_path + "/missing").config_files
+
+    def test_should_not_look_for_parent_directory_when_it_finds_valid_configurations(self):
+        self._create_file(self.test_files_path + '/../../settings.ini', '[settings]')
+        self._create_file(self.test_files_path + '/../../.env')
+        self._create_file(self.test_files_path + '/../.env')
+        self._create_file(self.test_files_path + '/../settings.ini', '[settings]')
+
+        discovery = ConfigurationDiscovery(os.path.dirname(self.test_files_path))
+        self.assertEqual(len(discovery.config_files), 2)
+        filenames = [cfg.filename for cfg in discovery.config_files]
+        self.assertIn(os.path.abspath(self.test_files_path + '/../.env'), filenames)
+        self.assertIn(os.path.abspath(self.test_files_path + '/../settings.ini'), filenames)
+
+    def test_should_look_for_parent_directory_when_it_finds_invalid_configurations(self):
+        self._create_file(self.test_files_path + '/../../settings.ini', '[settings]')
+        self._create_file(self.test_files_path + '/../../.env')
+        self._create_file(self.test_files_path + '/../invalid.cfg', '')
+        self._create_file(self.test_files_path + '/../settings.ini', '')
+
+        discovery = ConfigurationDiscovery(os.path.dirname(self.test_files_path))
+        self.assertEqual(len(discovery.config_files), 2)
+        filenames = [cfg.filename for cfg in discovery.config_files]
+        self.assertIn(os.path.abspath(self.test_files_path + '/../../.env'), filenames)
+        self.assertIn(os.path.abspath(self.test_files_path + '/../../settings.ini'), filenames)