added additional testing for the configuration, concurrent access and config reading, all tests work

Author: Byron

lib/git/config.py

@@ -23,8 +23,6 @@ def __new__(metacls, name, bases, clsdict):
 		Equip all base-class methods with a _needs_values decorator, and all non-const methods
 		with a _set_dirty_and_flush_changes decorator in addition to that.
 		"""
-		new_type = super(_MetaParserBuilder, metacls).__new__(metacls, name, bases, clsdict)
-		
 		mutating_methods = clsdict['_mutating_methods_']
 		for base in bases:
 			methods = ( t for t in inspect.getmembers(base, inspect.ismethod) if not t[0].startswith("_") )
@@ -35,9 +33,11 @@ def __new__(metacls, name, bases, clsdict):
 				if name in mutating_methods:
 					method_with_values = _set_dirty_and_flush_changes(method_with_values)
 				# END mutating methods handling
+				
 				clsdict[name] = method_with_values
 		# END for each base
 
+		new_type = super(_MetaParserBuilder, metacls).__new__(metacls, name, bases, clsdict)
 		return new_type
 
 
@@ -98,7 +98,7 @@ class GitConfigParser(cp.RawConfigParser, object):
 		)
 
 	# list of RawConfigParser methods able to change the instance
-	_mutating_methods_ = ("remove_section", "remove_option", "set")
+	_mutating_methods_ = ("add_section", "remove_section", "remove_option", "set")
 
 	def __init__(self, file_or_files, read_only=True):
 		"""
@@ -141,8 +141,10 @@ def __del__(self):
 		"""
 		Write pending changes if required and release locks
 		"""
-		if self.read_only:
-			return 
+		# checking for the lock here makes sure we do not raise during write()
+		# in case an invalid parser was created who could not get a lock
+		if self.read_only or not self._has_lock():
+			return
 
 		try:
 			try:
@@ -242,7 +244,7 @@ def read(self):
 			close_fp = False
 			# assume a path if it is not a file-object
 			if not hasattr(file_object, "seek"):
-				fp = open(file_object, "w")
+				fp = open(file_object)
 				close_fp = True
 			# END fp handling
 
@@ -271,6 +273,7 @@ def write_section(name, section_dict):
 		map(lambda t: write_section(t[0],t[1]), self._sections.items())
 
 
+	@_needs_values
 	def write(self):
 		"""
 		Write changes to our file, if there are changes at all
@@ -307,6 +310,15 @@ def _assure_writable(self, method_name):
 		if self.read_only:
 			raise IOError("Cannot execute non-constant method %s.%s" % (self, method_name))
 
+	@_needs_values
+	@_set_dirty_and_flush_changes
+	def add_section(self, section):
+		"""
+		Assures added options will stay in order
+		"""
+		super(GitConfigParser, self).add_section(section)
+		self._sections[section] = OrderedDict()
+		
 	@property
 	def read_only(self):
 		"""

test/git/test_config.py

@@ -36,6 +36,29 @@ def test_read_write(self):
 			assert w_config._sections
 			w_config.write()				# enforce writing
 			assert file_obj.getvalue() == file_obj_orig.getvalue()
+			
+			# creating an additional config writer must fail due to exclusive access
+			self.failUnlessRaises(IOError, GitConfigParser, file_obj, read_only = False)
+			
+			# should still have a lock and be able to make changes
+			assert w_config._has_lock()
+			
+			# changes should be written right away
+			sname = "my_section"
+			oname = "mykey"
+			val = "myvalue"
+			w_config.add_section(sname)
+			assert w_config.has_section(sname)
+			w_config.set(sname, oname, val)
+			assert w_config.has_option(sname,oname)
+			assert w_config.get(sname, oname) == val
+			
+			file_obj.seek(0)
+			r_config = GitConfigParser(file_obj, read_only=True)
+			assert r_config.has_section(sname)
+			assert r_config.has_option(sname, oname)
+			assert r_config.get(sname, oname) == val
+			
 		# END for each filename
 
 	def test_base(self):
@@ -64,5 +87,3 @@ def test_base(self):
 		assert num_sections and num_options
 		assert r_config._is_initialized == True
 
-		
-		self.fail("TODO: Base config writer testing")

test/git/test_repo.py

@@ -228,6 +228,11 @@ def test_config_reader(self):
 
 	def test_config_writer(self):
 		for config_level in self.repo.config_level:
-			writer = self.repo.config_writer(config_level)
-			assert not writer.read_only
+			try:
+				writer = self.repo.config_writer(config_level)
+				assert not writer.read_only
+			except IOError:
+				# its okay not to get a writer for some configuration files if we 
+				# have no permissions
+				pass 
 		# END for each config level