Add tests for 53f107d, related to issues #46 & #66

Author: AlexanderWells-diamond

tests/conftest.py

@@ -15,6 +15,10 @@
     sys.platform.startswith("win"), reason="Cothread doesn't work on windows"
 )
 
+# Default length used to initialise Waveform and longString records.
+# Length picked to match string record length, so we can re-use test strings.
+WAVEFORM_LENGTH = 40
+
 class SubprocessIOC:
     def __init__(self, ioc_py):
         self.pv_prefix = "".join(
@@ -76,9 +80,9 @@ def _clear_records():
     # https://github.com/dls-controls/pythonSoftIOC/issues/56
     RecordLookup._RecordDirectory.clear()
 
-@pytest.fixture
+@pytest.fixture(autouse=True)
 def clear_records():
-    """Fixture to delete all records before and after a test."""
+    """Deletes all records before and after every test"""
     _clear_records()
     yield
     _clear_records()

tests/test_record_values.py

@@ -6,7 +6,7 @@
 from enum import Enum
 from math import isnan, inf, nan
 
-from conftest import requires_cothread
+from conftest import requires_cothread, WAVEFORM_LENGTH
 
 from softioc import asyncio_dispatcher, builder, softioc
 from softioc.pythonSoftIoc import RecordWrapper
@@ -18,6 +18,9 @@
 DEVICE_NAME = "RECORD-VALUE-TESTS"
 TIMEOUT = 5  # Seconds
 
+# The maximum length string for StringIn/Out records
+MAX_LEN_STR = "a 39 char string exactly maximum length"
+
 VERY_LONG_STRING = "This is a fairly long string, the kind that someone " \
     "might think to put into a record that can theoretically hold a huge " \
     "string and so lets test it and prove that shall we?"
@@ -101,6 +104,8 @@ def record_values_names(fixture_value):
     ("mbbOut_int", builder.mbbOut, 1, 1, int),
     ("strIn_abc", builder.stringIn, "abc", "abc", str),
     ("strOut_abc", builder.stringOut, "abc", "abc", str),
+    ("strIn_39chars", builder.stringIn, MAX_LEN_STR, MAX_LEN_STR, str),
+    ("strOut_39chars", builder.stringOut, MAX_LEN_STR, MAX_LEN_STR, str),
     ("strIn_empty", builder.stringIn, "", "", str),
     ("strOut_empty", builder.stringOut, "", "", str),
     ("strin_utf8", builder.stringIn, "%a€b", "%a€b", str),  # Valid UTF-8
@@ -310,7 +315,7 @@ def run_ioc(record_configurations: list, conn, set_enum, get_enum):
         if set_enum == SetValueEnum.INITIAL_VALUE:
             kwarg.update({"initial_value": initial_value})
         elif creation_func in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
             # Related to this issue:
             # https://github.com/dls-controls/pythonSoftIOC/issues/37
 
@@ -493,7 +498,7 @@ class TestGetValue:
     """Tests that use .get() to check whether values applied with .set(),
     initial_value, or caput return the expected value"""
 
-    def test_value_pre_init_set(self, clear_records, record_values):
+    def test_value_pre_init_set(self, record_values):
         """Test that records provide the expected values on get calls when using
         .set() and .get() before IOC initialisation occurs"""
 
@@ -507,7 +512,7 @@ def test_value_pre_init_set(self, clear_records, record_values):
 
         kwarg = {}
         if creation_func in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         out_rec = creation_func(record_name, **kwarg)
         out_rec.set(initial_value)
@@ -678,15 +683,15 @@ class TestDefaultValue:
         ],
     )
     def test_value_default_pre_init(
-        self, creation_func, expected_value, expected_type, clear_records
+        self, creation_func, expected_value, expected_type
     ):
         """Test that the correct default values are returned from .get() (before
         record initialisation) when no initial_value or .set() is done"""
         # Out records do not have default values until records are initialized
 
         kwarg = {}
         if creation_func in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         out_rec = creation_func("out-record", **kwarg)
         record_value_asserts(
@@ -728,7 +733,7 @@ def record_func_reject_none(self, record_func):
         return record_func
 
     def test_value_none_rejected_initial_value(
-        self, clear_records, record_func_reject_none
+        self, record_func_reject_none
     ):
         """Test setting \"None\" as the initial_value raises an exception"""
 
@@ -737,7 +742,7 @@ def test_value_none_rejected_initial_value(
             builder.WaveformIn,
             builder.WaveformOut,
         ]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         with pytest.raises(self.expected_exceptions):
             record_func_reject_none("SOME-NAME", initial_value=None, **kwarg)
@@ -749,7 +754,7 @@ def test_value_none_rejected_set_before_init(
 
         kwarg = {}
         if record_func_reject_none in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         with pytest.raises(self.expected_exceptions):
             record = record_func_reject_none("SOME-NAME", **kwarg)
@@ -759,7 +764,7 @@ def none_value_test_func(self, record_func, queue):
         """Start the IOC and catch the expected exception"""
         kwarg = {}
         if record_func in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         record = record_func("SOME-NAME", **kwarg)
 
@@ -793,3 +798,84 @@ def test_value_none_rejected_set_after_init(self, record_func_reject_none):
         finally:
             process.terminate()
             process.join(timeout=3)
+
+
+class TestInvalidValues:
+    """Tests for values that records should reject"""
+
+    def test_string_rejects_overlong_strings(self):
+        """Test that stringIn & stringOut records reject strings >=39 chars"""
+
+        OVERLONG_STR = MAX_LEN_STR + "A"
+
+        with pytest.raises(ValueError):
+            builder.stringIn("STRIN1", initial_value=OVERLONG_STR)
+
+        with pytest.raises(ValueError):
+            builder.stringOut("STROUT1", initial_value=OVERLONG_STR)
+
+        with pytest.raises(ValueError):
+            si = builder.stringIn("STRIN2")
+            si.set(OVERLONG_STR)
+
+        with pytest.raises(ValueError):
+            so = builder.stringOut("STROUT2", initial_value=OVERLONG_STR)
+            so.set(OVERLONG_STR)
+
+    def test_long_string_rejects_overlong_strings(self):
+        """Test that longStringIn & longStringOut records reject
+        strings >=39 chars"""
+        OVERLONG_STR = MAX_LEN_STR + "A"
+
+        with pytest.raises(AssertionError):
+            builder.longStringIn(
+                "LSTRIN1",
+                initial_value=OVERLONG_STR,
+                length=WAVEFORM_LENGTH)
+
+        with pytest.raises(AssertionError):
+            builder.longStringOut(
+                "LSTROUT1",
+                initial_value=OVERLONG_STR,
+                length=WAVEFORM_LENGTH)
+
+        with pytest.raises(AssertionError):
+            lsi = builder.longStringIn("LSTRIN2", length=WAVEFORM_LENGTH)
+            lsi.set(OVERLONG_STR)
+
+        with pytest.raises(AssertionError):
+            lso = builder.longStringIn("LSTROUT2", length=WAVEFORM_LENGTH)
+            lso.set(OVERLONG_STR)
+
+        # And a different way to initialise the records to trigger same behaviour:
+        with pytest.raises(AssertionError):
+            lsi = builder.longStringIn("LSTRIN3", initial_value="ABC")
+            lsi.set(OVERLONG_STR)
+
+        with pytest.raises(AssertionError):
+            lso = builder.longStringOut("LSTROUT3", initial_value="ABC")
+            lso.set(OVERLONG_STR)
+
+
+    def test_waveform_rejects_zero_length(self):
+        """Test that WaveformIn/Out and longStringIn/Out records throw an
+        exception when being initialized with a zero length array"""
+        with pytest.raises(AssertionError):
+            builder.WaveformIn("W_IN", [])
+        with pytest.raises(AssertionError):
+            builder.WaveformOut("W_OUT", [])
+        with pytest.raises(AssertionError):
+            builder.longStringIn("L_IN", length=0)
+        with pytest.raises(AssertionError):
+            builder.longStringOut("L_OUT", length=0)
+
+    def test_waveform_rejects_overlonglong_values(self):
+        """Test that Waveform records throw an exception when an overlong
+        value is written"""
+        w_in = builder.WaveformIn("W_IN", [1, 2, 3])
+        w_out = builder.WaveformOut("W_OUT", [1, 2, 3])
+
+        with pytest.raises(AssertionError):
+            w_in.set([1, 2, 3, 4])
+        with pytest.raises(AssertionError):
+            w_out.set([1, 2, 3, 4])

tests/test_records.py

@@ -4,7 +4,7 @@
 import pytest
 import asyncio
 
-from conftest import requires_cothread, _clear_records
+from conftest import requires_cothread, _clear_records, WAVEFORM_LENGTH
 
 from softioc import asyncio_dispatcher, builder, softioc
 
@@ -14,7 +14,7 @@
 DEVICE_NAME = "RECORD-TESTS"
 TIMEOUT = 5  # Seconds
 
-def test_records(tmp_path, clear_records):
+def test_records(tmp_path):
     # Ensure we definitely unload all records that may be hanging over from
     # previous tests, then create exactly one instance of expected records.
     from sim_records import create_records
@@ -85,32 +85,6 @@ def test_DISP_can_be_overridden():
     # Note: DISP attribute won't exist if field not specified
     assert record.DISP.Value() == 0
 
-def test_waveform_disallows_zero_length(clear_records):
-    """Test that WaveformIn/Out records throw an exception when being
-    initialized with a zero length array"""
-    with pytest.raises(AssertionError):
-        builder.WaveformIn("W_IN", [])
-    with pytest.raises(AssertionError):
-        builder.WaveformOut("W_OUT", [])
-
-def test_waveform_disallows_too_long_values(clear_records):
-    """Test that Waveform and longString records throw an exception when
-    an overlong value is written"""
-    w_in = builder.WaveformIn("W_IN", [1, 2, 3])
-    w_out = builder.WaveformOut("W_OUT", [1, 2, 3])
-
-    ls_in = builder.longStringIn("LS_IN", initial_value="ABC")
-    ls_out = builder.longStringOut("LS_OUT", initial_value="ABC")
-
-    with pytest.raises(AssertionError):
-        w_in.set([1, 2, 3, 4])
-    with pytest.raises(AssertionError):
-        w_out.set([1, 2, 3, 4])
-    with pytest.raises(AssertionError):
-        ls_in.set("ABCD")
-    with pytest.raises(AssertionError):
-        ls_out.set("ABCD")
-
 def validate_fixture_names(params):
     """Provide nice names for the out_records fixture in TestValidate class"""
     return params[0].__name__
@@ -149,7 +123,7 @@ def validate_ioc_test_func(self, record_func, queue, validate_pass: bool):
 
         kwarg = {}
         if record_func in [builder.WaveformIn, builder.WaveformOut]:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         kwarg.update(
             {
@@ -275,7 +249,7 @@ def on_update_func(new_val):
 
         kwarg = {}
         if record_func is builder.WaveformOut:
-            kwarg = {"length": 50}  # Required when no value on creation
+            kwarg = {"length": WAVEFORM_LENGTH}  # Must specify when no value
 
         record_func(
             "ON-UPDATE-RECORD",