[JGIT PATCH 2/2] Extended test cases for RepositoryConfigTest

Shawn O. Pearce· Jul 27, 2008, 02:11 UTC · lore

[JGIT PATCH 1/2] Fix RepositoryConfig.setStringList to not corrupt internal state

Calling setStringList was incorrectly storing the String objects directly in byName. The byName table should have either an Entry or a List<Entry> stored within its value position. Storing the String (or List<String>) directly confused our get code as it did not find the object type it expected. This caused the get code to fallback to the base configuration (e.g. ~/.gitconfig) or just return null (claiming the value was never set).

A test case for this appears in the next commit.

Noticed-by: Marek Zawirski <marek.zawirski@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   23 ++++++++++++++++---
 1 files changed, 19 insertions(+), 4 deletions(-)

Show changes to org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +19 −4

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index 1431f1f..d1cd5fc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -381,10 +381,25 @@ public class RepositoryConfig {
 		key += "." + name.toLowerCase();
 		if (values.size() == 0)
 			byName.remove(key);
-		else if (values.size() == 1)
-			byName.put(key, values.get(0));
-		else
-			byName.put(key, new ArrayList<String>(values));
+		else if (values.size() == 1) {
+			final Entry e = new Entry();
+			e.base = section;
+			e.extendedBase = subsection;
+			e.name = name;
+			e.value = values.get(0);
+			byName.put(key, e);
+		} else {
+			final ArrayList<Entry> eList = new ArrayList<Entry>(values.size());
+			for (final String v : values) {
+				final Entry e = new Entry();
+				e.base = section;
+				e.extendedBase = subsection;
+				e.name = name;
+				e.value = v;
+				eList.add(e);
+			}
+			byName.put(key, eList);
+		}
 
 		int entryIndex = 0;
 		int valueIndex = 0;

-- 
1.6.0.rc0.182.gb96c7

Shawn O. Pearce· Jul 27, 2008, 02:11 UTC · re: Shawn O. Pearce · lore

From: Marek Zawirski <marek.zawirski@gmail.com>

Add cases for getting values after setting them. It's something different from reading config file from scratch or writing it out after modifications. It's case for getting values after modifications in object model.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/lib/RepositoryConfigTest.java |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

Show changes to org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +19 −1

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
index 8c55fe8..da7e704 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
@@ -2,6 +2,7 @@
  * Copyright (C) 2007, Dave Watson <dwatson@mimvista.com>
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  *
  * All rights reserved.
  *
@@ -41,6 +42,8 @@ package org.spearce.jgit.lib;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.LinkedList;
 
 /**
  * Test reading of git config
@@ -84,11 +87,26 @@ public class RepositoryConfigTest extends RepositoryTestCase {
 		checkFile(cfgFile, "[sec \"ext\"]\n\tname = value\n\tname2 = value2\n");
 	}
 
-	public void test004_PutSimple() throws IOException {
+	public void test004_PutGetSimple() throws IOException {
 		File cfgFile = writeTrashFile("config_004", "");
 		RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile);
 		repositoryConfig.setString("my", null, "somename", "false");
 		repositoryConfig.save();
 		checkFile(cfgFile, "[my]\n\tsomename = false\n");
+		assertEquals("false", repositoryConfig
+				.getString("my", null, "somename"));
+	}
+
+	public void test005_PutGetStringList() throws IOException {
+		File cfgFile = writeTrashFile("config_005", "");
+		RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile);
+		final LinkedList<String> values = new LinkedList<String>();
+		values.add("value1");
+		values.add("value2");
+		repositoryConfig.setStringList("my", null, "somename", values);
+		repositoryConfig.save();
+		assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+				.getStringList("my", null, "somename")));
+		checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
 	}
 }

-- 
1.6.0.rc0.182.gb96c7

patch, 2 partsExtended test cases for RepositoryConfigTest