From d045115d97fdb94c84a7c7f7fd2fefdb95004d19 Mon Sep 17 00:00:00 2001
From: Josh McVey <josh.mcvey@opentrons.com>
Date: Wed, 4 Sep 2024 10:27:03 -0500
Subject: [PATCH] fix(robot-server): ignore trailing empty lines in csv rtps
 (#16185)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Overview

<https://opentrons.atlassian.net/browse/RQA-3134>

> Since it’s commonplace to put an empty line at the end of a file,
parse_as_csv() should ignore such lines.

## Test Plan and Hands on Testing

Unit tests cover the scenarios. Will test actual CSV file in bug
verification on the next alpha.

## Risk assessment

- Low other than this deviates from how the standard library
`csv.reader` behaves.
---
 .../parameters/csv_parameter_interface.py     |  7 ++
 .../test_csv_parameter_interface.py           | 69 +++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/api/src/opentrons/protocols/parameters/csv_parameter_interface.py b/api/src/opentrons/protocols/parameters/csv_parameter_interface.py
index a1b9e7b4df7..6da9a0f7aaf 100644
--- a/api/src/opentrons/protocols/parameters/csv_parameter_interface.py
+++ b/api/src/opentrons/protocols/parameters/csv_parameter_interface.py
@@ -84,4 +84,11 @@ def parse_as_csv(
                 rows.append(row)
         except (UnicodeDecodeError, csv.Error):
             raise ParameterValueError("Cannot parse provided CSV contents.")
+        return self._remove_trailing_empty_rows(rows)
+
+    @staticmethod
+    def _remove_trailing_empty_rows(rows: List[List[str]]) -> List[List[str]]:
+        """Removes any trailing empty rows."""
+        while rows and rows[-1] == []:
+            rows.pop()
         return rows
diff --git a/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py b/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py
index 81bffd0028e..f0bf7d89f32 100644
--- a/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py
+++ b/api/tests/opentrons/protocols/parameters/test_csv_parameter_interface.py
@@ -1,3 +1,4 @@
+from typing import List, Tuple
 import pytest
 import platform
 from decoy import Decoy
@@ -44,6 +45,42 @@ def csv_file_different_delimiter() -> bytes:
     return b"x:y:z\na,:1,:2\nb,:3,:4\nc,:5,:6"
 
 
+@pytest.fixture
+def csv_file_basic_trailing_empty() -> Tuple[bytes, List[List[str]]]:
+    """A basic CSV file with quotes around strings and a trailing newline."""
+    return (
+        b'"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6\n',
+        [["x", "y", "z"], ["a", "1", "2"], ["b", "3", "4"], ["c", "5", "6"]],
+    )
+
+
+@pytest.fixture
+def csv_file_basic_three_trailing_empty() -> Tuple[bytes, List[List[str]]]:
+    """A basic CSV file with quotes around strings and three trailing newlines."""
+    return (
+        b'"x","y","z"\n"a",1,2\n"b",3,4\n"c",5,6\n\n\n',
+        [["x", "y", "z"], ["a", "1", "2"], ["b", "3", "4"], ["c", "5", "6"]],
+    )
+
+
+@pytest.fixture
+def csv_file_empty_row_and_trailing_empty() -> Tuple[bytes, List[List[str]]]:
+    """A basic CSV file with quotes around strings, an empty row, and a trailing newline."""
+    return (
+        b'"x","y","z"\n\n"b",3,4\n"c",5,6\n',
+        [["x", "y", "z"], [], ["b", "3", "4"], ["c", "5", "6"]],
+    )
+
+
+@pytest.fixture
+def csv_file_windows_empty_row_trailing_empty() -> Tuple[bytes, List[List[str]]]:
+    """A basic CSV file with quotes around strings, Windows-style newlines, an empty row, and a trailing newline."""
+    return (
+        b'"x","y","z"\r\n\r\n"b",3,4\r\n"c",5,6\r\n',
+        [["x", "y", "z"], [], ["b", "3", "4"], ["c", "5", "6"]],
+    )
+
+
 def test_csv_parameter(
     decoy: Decoy, api_version: APIVersion, csv_file_basic: bytes
 ) -> None:
@@ -125,3 +162,35 @@ def test_csv_parameter_dont_detect_dialect(
 
     assert rows[0] == ["x", ' "y"', ' "z"']
     assert rows[1] == ["a", " 1", " 2"]
+
+
+@pytest.mark.parametrize(
+    "csv_file_fixture",
+    [
+        "csv_file_basic_trailing_empty",
+        "csv_file_basic_three_trailing_empty",
+        "csv_file_empty_row_and_trailing_empty",
+        "csv_file_windows_empty_row_trailing_empty",
+    ],
+)
+def test_csv_parameter_trailing_empties(
+    decoy: Decoy,
+    api_version: APIVersion,
+    request: pytest.FixtureRequest,
+    csv_file_fixture: str,
+) -> None:
+    """It should load the rows as all strings.  Empty rows are allowed in the middle of the data but all trailing empty rows are removed."""
+    # Get the fixture value
+    csv_file: bytes
+    expected_output: List[List[str]]
+    csv_file, expected_output = request.getfixturevalue(csv_file_fixture)
+
+    subject = CSVParameter(csv_file, api_version)
+    parsed_csv = subject.parse_as_csv()
+
+    assert (
+        parsed_csv == expected_output
+    ), f"Expected {expected_output}, but got {parsed_csv}"
+    assert len(parsed_csv) == len(
+        expected_output
+    ), f"Expected {len(expected_output)} rows, but got {len(parsed_csv)}"