Committing patches by Iain:

- use OpenCSV parser instead of our own
 - use TestNG instead of JUnit which is a lot more configurable in test selection (and allows us to do a much better job a leaving the tree green even while developing tests that are known to fail)
 - integrated TestNG in './gridworks test'
 - added Iain to the list of contributors in README.txt
 - changed the Eclipse test launch file to use the TestNG launcher (unfortunately, this is not shipped by default in Eclipse, so you have to install it yourself from the http://beust.com/eclipse update file, I'll add this to the wiki shortly)


git-svn-id: http://google-refine.googlecode.com/svn/trunk@782 7d457c2a-affb-35e4-300a-418c747d4874

.classpath

@@ -24,13 +24,14 @@
 	<classpathentry kind="lib" path="lib/httpclient-4.0.1.jar" sourcepath="lib-src/httpclient-4.0.1-sources.jar"/>
 	<classpathentry kind="lib" path="lib/signpost-core-1.2.1.1.jar" sourcepath="lib-src/signpost-core-1.2.1.1-sources.jar"/>
 	<classpathentry kind="lib" path="lib/signpost-commonshttp4-1.2.1.1.jar" sourcepath="lib-src/signpost-commonshttp4-1.2.1.1-sources.jar"/>
+	<classpathentry kind="lib" path="lib/opencsv-2.2.jar" sourcepath="tests/java/lib-src/opencsv-2.2-sources.jar"/>
 	<classpathentry kind="lib" path="lib/poi-3.6.jar"/>
 	<classpathentry kind="lib" path="lib/poi-ooxml-3.6.jar"/>
 	<classpathentry kind="lib" path="lib/jython-2.5.1.jar"/>
 	<classpathentry kind="lib" path="lib/clojure-1.1.0.jar"/>
 	<classpathentry kind="lib" path="lib/jackson-core-asl-1.5.1.jar"/>
 	<classpathentry kind="lib" path="lib/marc4j-2.4.jar"/>
-	<classpathentry kind="lib" path="tests/java/lib/junit-4.8.1.jar" sourcepath="tests/java/lib-src/junit-4.8.1-sources.jar"/>
 	<classpathentry kind="lib" path="tests/java/lib/mockito-all-1.8.4.jar" sourcepath="tests/java/lib-src/mockito-all-1.8.4-sources.jar"/>
+	<classpathentry kind="lib" path="tests/java/lib/testng-5.12.1.jar" sourcepath="tests/java/lib-src/testng-5.12.1-sources.jar"/>
 	<classpathentry kind="output" path="src/main/webapp/WEB-INF/classes"/>
 </classpath>

LICENSE.txt

@@ -55,6 +55,8 @@ licenses/apache2.0.LICENSE.txt
     servlet-api
     xmlbeans
     signpost
+    opencsv
+    textng
    
 LGPL
 ----

README.txt

@@ -48,7 +48,7 @@
    - Jeff Fry <jfry@metaweb.com>
    - Will Moffat <willmoffat@metaweb.com>
    - James Home <jh@metaweb.com>
-  
+   - Iain Sproat <iainsproat@gmail.com>
      
 
                                   - o -

gridworks

@@ -529,14 +529,12 @@ server_test() {
     CLASSPATH="$GRIDWORKS_TEST_DIR/java/classes${SEP}$GRIDWORKS_WEBAPP/WEB-INF/classes${SEP}$GRIDWORKS_BUILD_DIR/classes${SEP}$GRIDWORKS_TEST_DIR/java/lib/*${SEP}$GRIDWORKS_LIB_DIR/*${SEP}$GRIDWORKS_WEBAPP/WEB-INF/lib/*"
 
     if [ -z "$1" ]; then
-        cd "$GRIDWORKS_TEST_DIR/java/classes"
+        TESTS="-excludegroups broken $GRIDWORKS_TEST_DIR/java/conf/tests.xml"
-        TESTS=`find . -name '*.class' | sed s@/@.@g | sed s@.class@@ | sed s@..@@`
-        cd ../../..
     else 
-        TESTS=$1
+        TESTS="-testclass $1"
     fi
     
-    RUN_CMD="$JAVA -cp $CLASSPATH $OPTS org.junit.runner.JUnitCore $TESTS"
+    RUN_CMD="$JAVA -cp $CLASSPATH $OPTS org.testng.TestNG -d $GRIDWORKS_BUILD_DIR/server_tests -listener org.testng.reporters.DotTestListener $TESTS"
     
     #echo "$RUN_CMD"
     #echo ""

ide/eclipse/GridworksTests.launch

@@ -1,19 +1,21 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<launchConfiguration type="org.eclipse.jdt.junit.launchconfig">
+<launchConfiguration type="org.testng.eclipse.launchconfig">
-<stringAttribute key="bad_container_name" value="/gridworks."/>
+<booleanAttribute key="com.mountainminds.eclemma.core.INPLACE_INSTRUMENTATION" value="false"/>
-<listAttribute key="org.eclipse.debug.core.MAPPED_RESOURCE_PATHS">
+<listAttribute key="com.mountainminds.eclemma.core.INSTRUMENTATION_PATHS">
-<listEntry value="/gridworks"/>
+<listEntry value="/gridworks/src/main/webapp/WEB-INF/classes"/>
 </listAttribute>
-<listAttribute key="org.eclipse.debug.core.MAPPED_RESOURCE_TYPES">
+<stringAttribute key="org.eclipse.jdt.launching.MAIN_TYPE" value="org.testng.remote.RemoteTestNG"/>
-<listEntry value="4"/>
-</listAttribute>
-<mapAttribute key="org.eclipse.debug.core.preferred_launchers">
-<mapEntry key="[run]" value="org.eclipse.jdt.junit.launchconfig"/>
-</mapAttribute>
-<stringAttribute key="org.eclipse.jdt.junit.CONTAINER" value="=gridworks"/>
-<booleanAttribute key="org.eclipse.jdt.junit.KEEPRUNNING_ATTR" value="false"/>
-<stringAttribute key="org.eclipse.jdt.junit.TESTNAME" value=""/>
-<stringAttribute key="org.eclipse.jdt.junit.TEST_KIND" value="org.eclipse.jdt.junit.loader.junit4"/>
-<stringAttribute key="org.eclipse.jdt.launching.MAIN_TYPE" value=""/>
 <stringAttribute key="org.eclipse.jdt.launching.PROJECT_ATTR" value="gridworks"/>
+<stringAttribute key="org.eclipse.jdt.launching.WORKING_DIRECTORY" value="${workspace_loc:gridworks/build}"/>
+<mapAttribute key="org.testng.eclipse.ALL_CLASS_METHODS"/>
+<listAttribute key="org.testng.eclipse.CLASS_TEST_LIST"/>
+<stringAttribute key="org.testng.eclipse.COMPLIANCE_LEVEL" value="JDK"/>
+<listAttribute key="org.testng.eclipse.GROUP_LIST"/>
+<listAttribute key="org.testng.eclipse.GROUP_LIST_CLASS"/>
+<stringAttribute key="org.testng.eclipse.LOG_LEVEL" value="2"/>
+<listAttribute key="org.testng.eclipse.PACKAGE_TEST_LIST"/>
+<listAttribute key="org.testng.eclipse.SUITE_TEST_LIST">
+<listEntry value="tests/java/conf/tests.xml"/>
+</listAttribute>
+<intAttribute key="org.testng.eclipse.TYPE" value="3"/>
 </launchConfiguration>

src/main/java/com/metaweb/gridworks/importers/ImporterUtilities.java

@@ -15,14 +15,16 @@ public class ImporterUtilities {
     static public Serializable parseCellValue(String text) {
         if (text.length() > 0) {
             if (text.length() > 1 && text.startsWith("\"") && text.endsWith("\"")) {
-                return text.substring(1, text.length() - 1);
+                return text.substring(1, text.length() - 1); // FIXME is this a good assumption?
+                // what about the following string: '"To be" is almost always followed by "or not to be"'.
+                // This would be emitted as: 'To be" is almost always followed by "or not to be'
             }
-            
+
             try {
                 return Long.parseLong(text);
             } catch (NumberFormatException e) {
             }
-        
+
             try {
                 double d = Double.parseDouble(text);
                 if (!Double.isInfinite(d) && !Double.isNaN(d)) {
@@ -33,7 +35,7 @@ public class ImporterUtilities {
         }
         return text;
     }
-    
+
     static public int getIntegerOption(String name, Properties options, int def) {
         int value = def;
         if (options.containsKey(name)) {
@@ -45,7 +47,7 @@ public class ImporterUtilities {
         }
         return value;
     }
-    
+
     static public boolean getBooleanOption(String name, Properties options, boolean def) {
         boolean value = def;
         if (options.containsKey(name)) {
@@ -60,28 +62,28 @@ public class ImporterUtilities {
 
     static public void appendColumnName(List<String> columnNames, int index, String name) {
         name = name.trim();
-        
+
         while (columnNames.size() <= index) {
             columnNames.add("");
         }
-        
+
         if (!name.isEmpty()) {
             String oldName = columnNames.get(index);
             if (!oldName.isEmpty()) {
                 name = oldName + " " + name;
             }
-            
+
             columnNames.set(index, name);
         }
     }
-    
+
     static public void ensureColumnsInRowExist(List<String> columnNames, Row row) {
         int count = row.cells.size();
         while (count > columnNames.size()) {
             columnNames.add("");
         }
     }
-    
+
     static public void setupColumns(Project project, List<String> columnNames) {
         Map<String, Integer> nameToIndex = new HashMap<String, Integer>();
         for (int c = 0; c < columnNames.size(); c++) {
@@ -91,20 +93,20 @@ public class ImporterUtilities {
             } else if (cell.startsWith("\"") && cell.endsWith("\"")) {
                 cell = cell.substring(1, cell.length() - 1).trim();
             }
-            
+
             if (nameToIndex.containsKey(cell)) {
                 int index = nameToIndex.get(cell);
                 nameToIndex.put(cell, index + 1);
-                
+
                 cell = cell.contains(" ") ? (cell + " " + index) : (cell + index);
             } else {
                 nameToIndex.put(cell, 2);
             }
-            
+
             Column column = new Column(c, cell);
-            
+
             project.columnModel.columns.add(column);
         }
     }
-    
+
 }

src/main/java/com/metaweb/gridworks/importers/TsvCsvImporter.java

@@ -1,97 +1,99 @@
 package com.metaweb.gridworks.importers;
 
+import java.io.IOException;
 import java.io.InputStream;
-import java.io.LineNumberReader;
 import java.io.Reader;
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
 
-import org.apache.commons.lang.StringUtils;
+import au.com.bytecode.opencsv.CSVReader;
 
-import com.metaweb.gridworks.importers.parsers.CSVRowParser;
+import com.metaweb.gridworks.expr.ExpressionUtils;
-import com.metaweb.gridworks.importers.parsers.NonSplitRowParser;
+import com.metaweb.gridworks.model.Cell;
-import com.metaweb.gridworks.importers.parsers.RowParser;
-import com.metaweb.gridworks.importers.parsers.SeparatorRowParser;
 import com.metaweb.gridworks.model.Project;
 import com.metaweb.gridworks.model.Row;
 
 public class TsvCsvImporter implements Importer {
     public void read(Reader reader, Project project, Properties options) throws Exception {
         boolean splitIntoColumns = ImporterUtilities.getBooleanOption("split-into-columns", options, true);
-        
+
         String sep = options.getProperty("separator"); // auto-detect if not present
         int ignoreLines = ImporterUtilities.getIntegerOption("ignore", options, -1);
         int headerLines = ImporterUtilities.getIntegerOption("header-lines", options, 1);
-        
+
         int limit = ImporterUtilities.getIntegerOption("limit",options,-1);
         int skip = ImporterUtilities.getIntegerOption("skip",options,0);
         boolean guessValueType = ImporterUtilities.getBooleanOption("guess-value-type", options, true);
+
+        // default expected format is CSV
+        char separator = (sep != null && sep.length() == 1 && splitIntoColumns) ? sep.toCharArray()[0] : ','; 
+
+        CSVReader CsvReader = new CSVReader(reader, separator);
+        read(CsvReader, project, limit, skip, ignoreLines, headerLines, guessValueType);
+    }
+
+    /**
+     *
+     * @param reader
+     * @param project
+     * @param limit - negative for no limit.
+     * @param skip
+     * @param ignoreLines
+     * @param headerLines
+     * @param guessValueType
+     * @throws IOException
+     */
+    public void read(CSVReader reader, Project project, int limit, int skip, int ignoreLines, int headerLines, boolean guessValueType ) throws IOException {
         
+        // prevent logic errors below when negative numbers are introduced by defaulting to zero (except limit which is negative to indicate no limit)
+        if (skip < 0) skip = 0;
+        if (ignoreLines < 0) ignoreLines = 0;
+        if (headerLines < 0) headerLines = 0;
+
         List<String> columnNames = new ArrayList<String>();
-                
+        String [] nextLine;
-        LineNumberReader lnReader = new LineNumberReader(reader);
+        int lineCounter = 0;
-        RowParser parser = (sep != null && sep.length() > 0 && splitIntoColumns) ? 
+        while ((nextLine = reader.readNext()) != null) {
-                new SeparatorRowParser(sep) : null;
+            lineCounter++;
-        
+
-        String line = null;
+            if (limit > 0 && lineCounter > limit + ignoreLines + headerLines + skip) break;
-        int rowsWithData = 0;
+            if (ignoreLines > 0 && lineCounter <= ignoreLines) continue; // initial non-blank lines
-        
+            if (headerLines > 0 && lineCounter <= ignoreLines + headerLines && lineCounter > ignoreLines) {
-        while ((line = lnReader.readLine()) != null) {
+                // deal with column headers
-            if (ignoreLines > 0) {
+                for (int c = 0; c < nextLine.length; c++) {
-                ignoreLines--;
+                    String cell = nextLine[c].trim();
-                continue;
-            } else if (StringUtils.isBlank(line)) {
-                continue;
-            }
-            
-            if (parser == null) {
-                if (splitIntoColumns) {
-                    int tab = line.indexOf('\t');
-                    if (tab >= 0) {
-                        sep = "\t";
-                        parser = new SeparatorRowParser(sep);
-                    } else {
-                        sep = ",";
-                        parser = new CSVRowParser();
-                    }
-                } else {
-                    parser = new NonSplitRowParser();
-                }
-            }
-            
-            if (headerLines > 0) {
-                headerLines--;
-                
-                List<String> cells = parser.split(line, lnReader);
-                for (int c = 0; c < cells.size(); c++) {
-                    String cell = cells.get(c).trim();
-                    
                     ImporterUtilities.appendColumnName(columnNames, c, cell);
                 }
             } else {
+                // a data line (or a line below the header)
+                if (skip > 0 && lineCounter <= ignoreLines + headerLines + skip) continue; // skip initial data lines
+
+                // data line
                 Row row = new Row(columnNames.size());
-                
+                project.rows.add(row);
-                if (parser.parseRow(row, line, guessValueType, lnReader)) {
+                project.columnModel.setMaxCellIndex(row.cells.size());
-                    rowsWithData++;
+                for (String s : nextLine) {
-                    
+                    Serializable value = guessValueType ? ImporterUtilities.parseCellValue(s) : s;
-                    if (skip <= 0 || rowsWithData > skip) {
+
-                        project.rows.add(row);
+                    if (ExpressionUtils.isNonBlankData(value)) {
-                        project.columnModel.setMaxCellIndex(row.cells.size());
+                        row.cells.add(new Cell(value, null));
-                        
+                    } else {
-                        ImporterUtilities.ensureColumnsInRowExist(columnNames, row);
+                        row.cells.add(null);
-                        
-                        if (limit > 0 && project.rows.size() >= limit) {
-                            break;
-                        }
                     }
                 }
+                ImporterUtilities.ensureColumnsInRowExist(columnNames, row);
             }
         }
-        
+
         ImporterUtilities.setupColumns(project, columnNames);
     }
 
+    protected void DealWithHeaders(String[] nextLine, List<String> columnNames){
+
+    }
+
     public void read(InputStream inputStream, Project project, Properties options) throws Exception {
         throw new UnsupportedOperationException();
     }

tests/java/conf/tests.xml

@@ -0,0 +1,9 @@
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
+ 
+<suite name="Gridworks Unit Tests">
+  <test name="tests">
+    <packages>
+      <package name="com.metaweb.gridworks.tests.*"/>
+    </packages>
+  </test>
+</suite>

tests/java/src/com/metaweb/gridworks/tests/GridworksServletStub.java

@@ -6,8 +6,6 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.junit.Ignore;
-
 import com.metaweb.gridworks.GridworksServlet;
 import com.metaweb.gridworks.commands.Command;
 
@@ -15,8 +13,7 @@ import com.metaweb.gridworks.commands.Command;
  * Exposes protected methods of com.metaweb.gridworks.GridworksServlet as public for unit testing
  *
  */
-@Ignore
+public class GridworksServletStub extends GridworksServlet {
-public class GridworksServletStub extends GridworksServlet{
 
     //requirement of extending HttpServlet, not required for testing
     private static final long serialVersionUID = 1L;

tests/java/src/com/metaweb/gridworks/tests/GridworksServletTests.java

@@ -11,12 +11,12 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
 
 import com.metaweb.gridworks.commands.Command;
 
@@ -38,9 +38,8 @@ public class GridworksServletTests {
     Command command = null;
 
 
-    @Before
+    @BeforeMethod
-    public void SetUp()
+    public void SetUp() {
-    {
         SUT = new GridworksServletStub();
         request = mock(HttpServletRequest.class);
         response = mock(HttpServletResponse.class);
@@ -49,14 +48,15 @@ public class GridworksServletTests {
         GridworksServletStub.InsertCommand(TEST_COMMAND_NAME,command); //inject mock into command container
     }
 
-    @After
+    @AfterMethod
-    public void TearDown(){
+    public void TearDown() {
         SUT = null;
         request = null;
         response = null;
         command = null;
         GridworksServletStub.RemoveCommand(TEST_COMMAND_NAME); //remove mock to clean command container
     }
+    
     //-------------------AutoSaveTimerTask tests-----------
     //TODO would need to mock Timer and inject it into GridworksServlet.  Also need to deal with ProjectManager.singleton
     //-------------------init tests------------------------

tests/java/src/com/metaweb/gridworks/tests/commands/CommandStub.java

@@ -5,7 +5,6 @@ import javax.servlet.http.HttpServletRequest;
 
 import org.json.JSONException;
 import org.json.JSONObject;
-import org.junit.Ignore;
 
 import com.metaweb.gridworks.browsing.Engine;
 import com.metaweb.gridworks.commands.Command;
@@ -14,7 +13,6 @@ import com.metaweb.gridworks.model.Project;
 /**
  * Implementation of abstract class for testing Exposes protected members as public
  */
-@Ignore
 public class CommandStub extends Command {
 
     public Project wrapGetProject(HttpServletRequest request)

tests/java/src/com/metaweb/gridworks/tests/commands/CommandTests.java

@@ -10,12 +10,12 @@ import javax.servlet.http.HttpServletRequest;
 
 import org.json.JSONException;
 import org.json.JSONObject;
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
 
 import com.metaweb.gridworks.ProjectManager;
 import com.metaweb.gridworks.browsing.Engine;
@@ -30,7 +30,7 @@ public class CommandTests {
     ProjectManager projectManager = null;
     Project project = null;
 
-    @Before
+    @BeforeMethod
     public void SetUp() {
         SUT = new CommandStub();
         request = mock(HttpServletRequest.class);
@@ -38,7 +38,7 @@ public class CommandTests {
         project = mock(Project.class);
     }
 
-    @After
+    @AfterMethod
     public void TearDown() {
         SUT = null;
         request = null;

tests/java/src/com/metaweb/gridworks/tests/commands/util/CancelProcessesCommandTests.java

@@ -13,12 +13,12 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
 
 import com.metaweb.gridworks.ProjectManager;
 import com.metaweb.gridworks.commands.util.CancelProcessesCommand;
@@ -45,7 +45,7 @@ public class CancelProcessesCommandTests {
     ProcessManager processMan = null;
     PrintWriter pw = null;
 
-    @Before
+    @BeforeMethod
     public void SetUp() {
         projMan = mock(ProjectManager.class);
         ProjectManager.singleton = projMan;
@@ -58,7 +58,7 @@ public class CancelProcessesCommandTests {
         SUT = new CancelProcessesCommand();
     }
 
-    @After
+    @AfterMethod
     public void TearDown() {
         SUT = null;
 

tests/java/src/com/metaweb/gridworks/tests/importers/ImporterUtilitiesTests.java

@@ -0,0 +1,121 @@
+package com.metaweb.gridworks.tests.importers;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.mock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import com.metaweb.gridworks.importers.ImporterUtilities;
+import com.metaweb.gridworks.model.Cell;
+import com.metaweb.gridworks.model.Project;
+import com.metaweb.gridworks.model.Row;
+
+public class ImporterUtilitiesTests {
+    // logging
+    final static protected Logger logger = LoggerFactory.getLogger("ImporterUtilitiesTests");
+
+    @Test(enabled=false)
+    public void parseCellValueWithText(){
+        String END_QUOTES_SHOULD_BE_RETAINED = "\"To be\" is almost always followed by \"or not to be\"";
+        String response = (String) ImporterUtilities.parseCellValue(END_QUOTES_SHOULD_BE_RETAINED);
+        Assert.assertEquals(response, END_QUOTES_SHOULD_BE_RETAINED);
+    }
+
+    @Test
+    public void getIntegerOption(){
+        Properties options = mock(Properties.class);
+        when(options.containsKey("testInteger")).thenReturn(true);
+        when(options.getProperty("testInteger")).thenReturn("5");
+        int response = ImporterUtilities.getIntegerOption("testInteger", options, -1);
+        Assert.assertEquals(5, response);
+        verify(options, times(1)).containsKey("testInteger");
+        verify(options, times(1)).getProperty("testInteger");
+    }
+
+    @Test
+    public void getIntegerOptionReturnsDefaultOnError(){
+        Properties options = mock(Properties.class);
+        when(options.containsKey("testInteger")).thenReturn(true);
+        when(options.getProperty("testInteger")).thenReturn("notAnInteger");
+        int response = ImporterUtilities.getIntegerOption("testInteger", options, -1);
+        Assert.assertEquals(-1, response);
+        verify(options, times(1)).containsKey("testInteger");
+        verify(options, times(1)).getProperty("testInteger");
+    }
+
+    @Test
+    public void appendColumnName(){
+        List<String> columnNames = new ArrayList<String>();
+
+
+        ImporterUtilities.appendColumnName(columnNames, 0, "foo");
+        ImporterUtilities.appendColumnName(columnNames, 1, "bar");
+        Assert.assertEquals(columnNames.size(), 2);
+        Assert.assertEquals(columnNames.get(0), "foo");
+        Assert.assertEquals(columnNames.get(1), "bar");
+    }
+
+    @Test
+    public void appendColumnNameFromMultipleRows(){
+        List<String> columnNames = new ArrayList<String>();
+
+        ImporterUtilities.appendColumnName(columnNames, 0, "foo");
+        ImporterUtilities.appendColumnName(columnNames, 0, "bar");
+        Assert.assertEquals(columnNames.size(), 1);
+        Assert.assertEquals(columnNames.get(0), "foo bar");
+    }
+
+    @Test
+    public void ensureColumnsInRowExist(){
+        String VALUE_1 = "value1";
+        String VALUE_2 = "value2";
+        Row row = new Row(2);
+        ArrayList<String> columnNames = new ArrayList<String>(2);
+        columnNames.add(VALUE_1);
+        columnNames.add(VALUE_2);
+
+        ImporterUtilities.ensureColumnsInRowExist(columnNames, row);
+
+        Assert.assertEquals(columnNames.size(), 2);
+        Assert.assertEquals(columnNames.get(0), VALUE_1);
+        Assert.assertEquals(columnNames.get(1), VALUE_2);
+    }
+
+    @Test
+    public void ensureColumnsInRowExistDoesExpand(){
+        Row row = new Row(4);
+        for(int i = 1; i < 5; i++)
+            row.cells.add(new Cell("value" + i, null));
+
+        ArrayList<String> columnNames = new ArrayList<String>(2);
+
+
+        ImporterUtilities.ensureColumnsInRowExist(columnNames, row);
+
+        Assert.assertEquals(row.cells.size(), 4);
+        Assert.assertEquals(columnNames.size(), 4);
+    }
+
+    @Test
+    public void setupColumns(){
+        Project project = new Project();
+        List<String> columnNames = new ArrayList<String>();
+        columnNames.add("col1");
+        columnNames.add("col2");
+        columnNames.add("");
+        ImporterUtilities.setupColumns(project, columnNames);
+        Assert.assertEquals( project.columnModel.columns.get(0).getName(), "col1" );
+        Assert.assertEquals( project.columnModel.columns.get(1).getName(), "col2" );
+        Assert.assertEquals( project.columnModel.columns.get(2).getName(), "Column");
+    }
+
+}

tests/java/src/com/metaweb/gridworks/tests/importers/TsvCsvImporterTests.java

@@ -0,0 +1,179 @@
+package com.metaweb.gridworks.tests.importers;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.util.Properties;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import au.com.bytecode.opencsv.CSVReader;
+
+import com.metaweb.gridworks.importers.TsvCsvImporter;
+import com.metaweb.gridworks.model.Project;
+
+public class TsvCsvImporterTests {
+ // logging
+    final static protected Logger logger = LoggerFactory.getLogger("TsvCsvImporterTests");
+
+    //constants
+    String SAMPLE_ROW = "NDB_No,Shrt_Desc,Water";
+
+    //System Under Test
+    TsvCsvImporter SUT = null;
+
+    //mock dependencies
+    Project project = null;
+    Properties properties = null;
+
+    @BeforeMethod
+    public void SetUp(){
+        SUT = new TsvCsvImporter();
+        project = new Project(); //FIXME - should we try and use mock(Project.class); - seems unnecessary complexity
+        properties = mock(Properties.class);
+    }
+
+    @AfterMethod
+    public void TearDown(){
+        SUT = null;
+        project = null;
+        properties = null;
+    }
+    
+    @Test
+    public void readJustColumns(){
+        String input = "col1,col2,col3";
+        CSVReader reader = new CSVReader(new StringReader(input));
+        
+        try {
+            SUT.read(reader, project, -1, 0, 0, 1, false);
+        } catch (IOException e) {
+            Assert.fail();
+        }
+        Assert.assertEquals(project.columnModel.columns.size(), 3);
+        Assert.assertEquals(project.columnModel.columns.get(0).getName(), "col1");
+        Assert.assertEquals(project.columnModel.columns.get(1).getName(), "col2");
+        Assert.assertEquals(project.columnModel.columns.get(2).getName(), "col3");
+    }
+    
+    @Test
+    public void readSimpleData_1Header_1Row(){
+        String input = "col1,col2,col3\n" +
+                       "data1,data2,data3";
+        CSVReader reader = new CSVReader(new StringReader(input));
+        try {
+            SUT.read(reader, project, -1, 0, 0, 1, false);
+        } catch (IOException e) {
+            Assert.fail();
+        }
+        Assert.assertEquals(project.columnModel.columns.size(), 3);
+        Assert.assertEquals(project.columnModel.columns.get(0).getName(), "col1");
+        Assert.assertEquals(project.columnModel.columns.get(1).getName(), "col2");
+        Assert.assertEquals(project.columnModel.columns.get(2).getName(), "col3");
+        Assert.assertEquals(project.rows.size(), 1);
+        Assert.assertEquals(project.rows.get(0).cells.size(), 3);
+        Assert.assertEquals(project.rows.get(0).cells.get(0).value, "data1");
+        Assert.assertEquals(project.rows.get(0).cells.get(1).value, "data2");
+        Assert.assertEquals(project.rows.get(0).cells.get(2).value, "data3");
+    }
+    
+    @Test
+    public void readSimpleData_RowLongerThanHeader(){
+        String input = "col1,col2,col3\n" +
+        "data1,data2,data3,data4,data5,data6";
+        CSVReader reader = new CSVReader(new StringReader(input));
+        try {
+            SUT.read(reader, project, -1, 0, 0, 1, false);
+        } catch (IOException e) {
+            Assert.fail();
+        }
+        Assert.assertEquals(project.columnModel.columns.size(), 6);
+        Assert.assertEquals(project.columnModel.columns.get(0).getName(), "col1");
+        Assert.assertEquals(project.columnModel.columns.get(1).getName(), "col2");
+        Assert.assertEquals(project.columnModel.columns.get(2).getName(), "col3");
+        Assert.assertEquals(project.columnModel.columns.get(3).getName(), "Column");
+        Assert.assertEquals(project.columnModel.columns.get(3).getName(), "Column");
+        Assert.assertEquals(project.columnModel.columns.get(3).getName(), "Column");
+        Assert.assertEquals(project.rows.size(), 1);
+        Assert.assertEquals(project.rows.get(0).cells.size(), 6);
+        Assert.assertEquals(project.rows.get(0).cells.get(0).value, "data1");
+        Assert.assertEquals(project.rows.get(0).cells.get(1).value, "data2");
+        Assert.assertEquals(project.rows.get(0).cells.get(2).value, "data3");
+        Assert.assertEquals(project.rows.get(0).cells.get(3).value, "data4");
+        Assert.assertEquals(project.rows.get(0).cells.get(4).value, "data5");
+        Assert.assertEquals(project.rows.get(0).cells.get(5).value, "data6");
+    }
+    
+    @Test(enabled = false, groups = { "broken" })
+    public void readQuotedData(){
+        String input = "col1,col2,col3\n" +
+                       "\"To Be\" is often followed by \"or not To Be\",data2";
+        CSVReader reader = new CSVReader(new StringReader(input));
+        try {
+            SUT.read(reader, project, -1, 0, 0, 1, false);
+        } catch (IOException e) {
+            Assert.fail();
+        }
+        Assert.assertEquals(project.columnModel.columns.size(), 3);
+        Assert.assertEquals(project.columnModel.columns.get(0).getName(), "col1");
+        Assert.assertEquals(project.columnModel.columns.get(1).getName(), "col2");
+        Assert.assertEquals(project.columnModel.columns.get(2).getName(), "col3");
+        Assert.assertEquals(project.rows.size(), 1);
+        Assert.assertEquals(project.rows.get(0).cells.size(), 2);
+        Assert.assertEquals(project.rows.get(0).cells.get(0).value, "\"To Be\" is often followed by \"or not To Be\"");
+        Assert.assertEquals(project.rows.get(0).cells.get(1).value, "data2");
+    }
+
+    //---------------------read tests------------------------
+    @Test
+    public void readCsvWithProperties(){
+        StringReader reader = new StringReader(SAMPLE_ROW);
+
+        when(properties.getProperty("separator")).thenReturn(",");
+        whenGetIntegerOption("ignore",properties,0);
+        whenGetIntegerOption("header-lines",properties,0);
+        whenGetIntegerOption("limit",properties,-1);
+        whenGetIntegerOption("skip",properties,0);
+
+        try {
+            SUT.read(reader, project, properties);
+        } catch (Exception e) {
+            Assert.fail();
+        }
+
+
+        Assert.assertEquals(project.rows.size(), 1);
+        Assert.assertEquals(project.rows.get(0).cells.size(), 3);
+        Assert.assertEquals((String)project.rows.get(0).cells.get(0).value, "NDB_No");
+        Assert.assertEquals((String)project.rows.get(0).cells.get(1).value, "Shrt_Desc");
+        Assert.assertEquals((String)project.rows.get(0).cells.get(2).value, "Water");
+
+        verify(properties, times(1)).getProperty("separator");
+        verifyGetIntegerOption("ignore",properties);
+        verifyGetIntegerOption("header-lines",properties);
+        verifyGetIntegerOption("limit",properties);
+        verifyGetIntegerOption("skip",properties);
+    }
+
+    //--helpers--
+
+    public void whenGetIntegerOption(String name, Properties properties, int def){
+        when(properties.containsKey(name)).thenReturn(true);
+        when(properties.getProperty(name)).thenReturn(Integer.toString(def));
+    }
+
+    public void verifyGetIntegerOption(String name, Properties properties){
+        verify(properties, times(1)).containsKey(name);
+        verify(properties, times(1)).getProperty(name);
+    }
+
+}

tests/java/src/com/metaweb/gridworks/tests/importers/parsers/CSVRowParserTests.java

@@ -9,13 +9,12 @@ import java.io.IOException;
 import java.io.LineNumberReader;
 import java.util.List;
 
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Ignore;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
 
 import com.metaweb.gridworks.importers.parsers.CSVRowParser;
 
@@ -38,18 +37,20 @@ public class CSVRowParserTests {
     //mocked dependencies
     LineNumberReader lineReader = null;
 
-    @Before
+    @BeforeMethod
     public void SetUp(){
         lineReader = mock(LineNumberReader.class);
         SUT = new CSVRowParser();
     }
 
-    @After
+    @AfterMethod
     public void TearDown(){
         lineReader = null;
         SUT = null;
     }
+    
     //------------split tests-------------------------
+    
     @Test
     public void split(){
         List<String> splitLine = SUT.split(SAMPLE_ROW, lineReader);
@@ -68,16 +69,18 @@ public class CSVRowParserTests {
         Assert.assertEquals("15.87", splitLine.get(2));
     }
     
-    @Ignore("CSV parser not doing the right thing yet") @Test
+    @Test(enabled = false, groups = { "broken" })
     public void splitWithUnclosedQuote(){
         try {
-            when(lineReader.readLine()).thenReturn("");
+            when(lineReader.readLine()).thenReturn("continuation of row above, with comma\",value2");
         } catch (IOException e) {
             Assert.fail();
         }
         List<String> splitLine = SUT.split(UNCLOSED_QUOTED_ROW, lineReader);
         Assert.assertEquals(1, splitLine.size());
         Assert.assertEquals(UNCLOSED_QUOTED_ROW, splitLine.get(0));
+        Assert.assertEquals(UNCLOSED_QUOTED_ROW + "\ncontinuation of row above, with comma\"", splitLine.get(0));
+        Assert.assertEquals("value2", splitLine.get(1));
         
         try {
             verify(lineReader, times(1)).readLine();
@@ -86,7 +89,7 @@ public class CSVRowParserTests {
         }
     }
     
-    @Ignore("CSV parser not doing the right thing yet") @Test
+    @Test(enabled = false, groups = { "broken" })
     public void splitWithLeadingQuoteWithComma(){
         List<String> splitLine = SUT.split(LEADING_QUOTE_WITH_COMMA, lineReader);
         Assert.assertEquals(3, splitLine.size());
@@ -95,7 +98,7 @@ public class CSVRowParserTests {
         Assert.assertEquals("value3", splitLine.get(2));
     }
     
-    @Ignore("CSV parser not doing the right thing yet") @Test
+    @Test(enabled = false, groups = { "broken" })
     public void splitWithQuoteInsideValue(){
         List<String> splitLine = SUT.split(QUOTED, lineReader);
         Assert.assertEquals(3, splitLine.size());

tests/java/src/com/metaweb/gridworks/tests/util/ParsingUtilitiesTests.java

@@ -2,10 +2,10 @@ package com.metaweb.gridworks.tests.util;
 
 import org.json.JSONException;
 import org.json.JSONObject;
-import org.junit.Assert;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
 
 import com.metaweb.gridworks.util.ParsingUtilities;