Re: [commons-csv] 01/04: [CSV-248] Test the parser and map functionality after deserialization

[Alex Herbert]

On 21/01/2020 15:35, Gary Gregory wrote:


diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
b/src/main/java/org/apache/commons/csv/CSVRecord.java
index 471e94d..e32cd5a 100644
--- a/src/main/java/org/apache/commons/csv/CSVRecord.java
+++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
@@ -83,6 +83,12 @@ public final class CSVRecord implements Serializable,
Iterable<String> {
      /**
       * Returns a value by name.
       *
+     * <p>Note: This requires a field mapping obtained from the original
parser.
+     * A check using {@link #isMapped(String)} should be used to
determine if a
+     * mapping exists from the provide {@code name} to a field index. In
this case an
+     * exception will only be thrown if the record does not contain a
field corresponding
+     * to the mapping, that is the record length is not consistent with
the mapping size.
+     *
       * @param name


Please close all HTML tags.

Done in a later commit.

On 21/01/2020 15:37, Gary Gregory wrote:
+    @Test
+    public void testDeserialisation() throws IOException {
+        CSVRecord shortRec;
+        try (final CSVParser parser = CSVParser.parse("A,B\n#my
comment\nOne,Two", CSVFormat.DEFAULT.withHeader().withCommentMarker('#'))) {
+            shortRec = parser.iterator().next();
+        }
+        try (FileOutputStream out = new
FileOutputStream("/tmp/csvRecord.ser");
+            ObjectOutputStream oos = new ObjectOutputStream(out)) {
+            oos.writeObject(shortRec);
+        }
      }

  This can't be right: "/tmp/csvRecord.ser"

It wasn't meant to be there. It is how to create the serialisation 
binary file. I wrote it in version 1.8 then took the same code to 1.6 
and forgot to remove it from 1.8.

A later one removed this. When I noticed it in the commit summary I had 
already pushed and it was too late to rebase and fix up the commit history.


@@ -77,4 +77,4 @@ public class JiraCsv248Test {
      private static InputStream getTestInput() {
          return
ClassLoader.getSystemClassLoader().getResourceAsStream("CSV-248/csvRecord.bin");
      }
-}
\ No newline at end of file
+}
This has been fixed.

2) Should we have one file and test per version 1.7 back to 1.0?

You cannot test version 1.7 as that could not serialize the CSVRecord.

I tried 1.6 as that was the most recent. I suppose should have used 1.0. Any 
users wanting to use an intermediate release and find it to be broken for 
serialisation can upgrade to 1.8 where it should be fixed.

I've just generated the data using the code under tag CSV_1.0 which requires 
pre-1.7 code (not try with resources):

@Test
public void testDeserialisation() throws IOException {
    CSVRecord shortRec;
    CSVParser parser = CSVParser.parse("A,B\n#my comment\nOne,Two", 
CSVFormat.DEFAULT.withHeader().withCommentMarker('#'));
    shortRec = parser.iterator().next();
    parser.close();
    FileOutputStream out = new FileOutputStream("/tmp/csvRecord.ser");
    ObjectOutputStream oos = new ObjectOutputStream(out);
    oos.writeObject(shortRec);
    oos.close();
    out.close();
 }

The file is binary different from the current file. The test passes. Shall I 
commit the old serialised version?

Alex