Merge pull request #10 from imaginationtech/rand_len_list

Add sanitization for bad list lengths

Author: will-keen

constrainedrandom/internal/randvar.py

@@ -275,6 +275,7 @@ def set_rand_length(self, length: int) -> None:
         if self.rand_length is None:
             raise RuntimeError("RandVar was not marked as having a random length," \
                 " but set_rand_length was called.")
+        assert length >= 0, "length was not greater than or equal to zero"
         self.rand_length_val = length
 
     def _get_random(self) -> random.Random:

constrainedrandom/randobj.py

@@ -209,6 +209,8 @@ def my_fn(factor):
         # rand_length and length are mutually-exclusive.
         assert not ((length is not None) and (rand_length is not None)), \
             "length and rand_length are mutually-exclusive, but both were specified"
+        if length is not None:
+            assert length >= 0, "length was not greater than or equal to zero"
         if rand_length is not None:
             # Indicates the length of the RandVar depends on another random variable.
             assert rand_length in self._random_vars, f"random variable length '{name}' is not valid," \

tests/features/errors.py

@@ -23,7 +23,7 @@ class ImpossibleThorough(MultiSum):
     solver.
     '''
 
-    EXPECT_FAILURE = True
+    EXPECTED_ERROR_RAND = RandomizationError
 
     def get_randobj(self, *args):
         randobj = super().get_randobj(*args)
@@ -66,7 +66,7 @@ class ImpossibleComplexVar(testutils.RandObjTestBase):
     the variable state space is too large to fail on creation.
     '''
 
-    EXPECT_FAILURE = True
+    EXPECTED_ERROR_RAND = RandomizationError
 
     def get_randobj(self, *args):
         randobj = RandObj(*args)
@@ -81,7 +81,7 @@ class ImpossibleMultiVar(testutils.RandObjTestBase):
     Test an impossible constraint problem with multiple variables.
     '''
 
-    EXPECT_FAILURE = True
+    EXPECTED_ERROR_RAND = RandomizationError
 
     def get_randobj(self, *args):
         randobj = RandObj(*args)
@@ -93,3 +93,49 @@ def sum_gt_10(x, y):
             return x + y > 10
         randobj.add_constraint(sum_gt_10, ('a', 'b'))
         return randobj
+
+
+class NegativeLength(testutils.RandObjTestBase):
+    '''
+    Test a random list with negative length.
+    '''
+
+    EXPECTED_ERROR_INIT = AssertionError
+
+    def get_randobj(self, *args):
+        randobj = RandObj(*args)
+        randobj.add_rand_var('bad_list', bits=1, length=-1)
+        return randobj
+
+
+class NegativeRandLength(testutils.RandObjTestBase):
+    '''
+    Test a random list with negative random length.
+    '''
+
+    EXPECTED_ERROR_RAND = AssertionError
+
+    def get_randobj(self, *args):
+        randobj = RandObj(*args)
+        randobj.add_rand_var('bad_length', domain=range(-10,-1))
+        randobj.add_rand_var('bad_list', bits=1, rand_length='bad_length')
+        return randobj
+
+
+class EmptyListDependent(testutils.RandObjTestBase):
+    '''
+    Test a random length list which is empty
+    but depended on by another variable.
+    '''
+
+    EXPECTED_ERROR_RAND = RandomizationError
+
+    def get_randobj(self, *args):
+        randobj = RandObj(*args)
+        randobj.add_rand_var('length', domain=[0])
+        randobj.add_rand_var('list', bits=4, rand_length='length')
+        randobj.add_rand_var('list_member', bits=4)
+        def in_list_c(x, y):
+            return x in y
+        randobj.add_constraint(in_list_c, ('list_member', 'list'))
+        return randobj

tests/testutils.py

@@ -27,9 +27,14 @@ class RandObjTestBase(unittest.TestCase):
     Extend this class to create testcases.
     '''
 
+    # Number of iterations to do per call to `randomize_and_check_result`.
     ITERATIONS = 1000
+    # Global multiplier for test length, set via command line.
     TEST_LENGTH_MULTIPLIER = 1
-    EXPECT_FAILURE = False
+    # Expected exception when calling `get_randobj`.
+    EXPECTED_ERROR_INIT = None
+    # Expected exception when calling `randomize_and_check_result`.
+    EXPECTED_ERROR_RAND = None
 
     def setUp(self) -> None:
         self.iterations = self.ITERATIONS * self.TEST_LENGTH_MULTIPLIER
@@ -111,8 +116,8 @@ def randomize_and_check_result(
         Code to randomize a randobj and check its results against expected
         results.
         '''
-        if self.EXPECT_FAILURE:
-            self.assertRaises(RandomizationError, randobj.randomize)
+        if self.EXPECTED_ERROR_RAND is not None:
+            self.assertRaises(self.EXPECTED_ERROR_RAND, randobj.randomize)
         else:
             results = self.randomize_and_time(randobj, self.iterations)
             assertListOfDictsEqual(self, expected_results, results, "Non-determinism detected, results were not equal")
@@ -164,132 +169,145 @@ def test_randobj(self) -> None:
 
         # Test with seed 0
         r = random.Random(0)
-        randobj = self.get_randobj(r)
-        # Take a copy of the randobj for use later
-        randobj_copy = deepcopy(randobj)
-        if self.EXPECT_FAILURE:
-            self.assertRaises(RandomizationError, randobj.randomize)
+        if self.EXPECTED_ERROR_INIT is not None:
+            self.assertRaises(self.EXPECTED_ERROR_INIT, self.get_randobj, r)
         else:
-            results = self.randomize_and_time(randobj, self.iterations)
-            self.check(results)
-            if do_tmp_checks:
-                # Check when applying temporary constraints
-                tmp_results = self.randomize_and_time(randobj, self.iterations, tmp_constraints, tmp_values)
-                self.tmp_check(tmp_results)
-                # Check temporary constraints don't break base randomization
-                post_tmp_results = self.randomize_and_time(randobj, self.iterations)
-                self.check(post_tmp_results)
-                # Add temporary constraints permanently, see what happens
-                if tmp_constraints is not None:
-                    for constr, vars in tmp_constraints:
-                        randobj.add_constraint(constr, vars)
-                    add_results = self.randomize_and_time(randobj, self.iterations, tmp_values=tmp_values)
-                    self.tmp_check(add_results)
+            randobj = self.get_randobj(r)
+            # Take a copy of the randobj for use later
+            randobj_copy = deepcopy(randobj)
+            if self.EXPECTED_ERROR_RAND is not None:
+                self.assertRaises(self.EXPECTED_ERROR_RAND, randobj.randomize)
+            else:
+                results = self.randomize_and_time(randobj, self.iterations)
+                self.check(results)
+                if do_tmp_checks:
+                    # Check when applying temporary constraints
+                    tmp_results = self.randomize_and_time(randobj, self.iterations, tmp_constraints, tmp_values)
+                    self.tmp_check(tmp_results)
+                    # Check temporary constraints don't break base randomization
+                    post_tmp_results = self.randomize_and_time(randobj, self.iterations)
+                    self.check(post_tmp_results)
+                    # Add temporary constraints permanently, see what happens
+                    if tmp_constraints is not None:
+                        for constr, vars in tmp_constraints:
+                            randobj.add_constraint(constr, vars)
+                        add_results = self.randomize_and_time(randobj, self.iterations, tmp_values=tmp_values)
+                        self.tmp_check(add_results)
 
         # Test again with seed 0, ensuring results are the same.
         r0 = random.Random(0)
-        randobj0 = self.get_randobj(r0)
-        self.randomize_and_check_result(
-            randobj0,
-            results,
-            do_tmp_checks,
-            tmp_constraints,
-            tmp_values,
-            tmp_results,
-            post_tmp_results,
-            add_results,
-            add_tmp_constraints=True,
-        )
+        if self.EXPECTED_ERROR_INIT is not None:
+            self.assertRaises(self.EXPECTED_ERROR_INIT, self.get_randobj, r0)
+        else:
+            randobj0 = self.get_randobj(r0)
+            self.randomize_and_check_result(
+                randobj0,
+                results,
+                do_tmp_checks,
+                tmp_constraints,
+                tmp_values,
+                tmp_results,
+                post_tmp_results,
+                add_results,
+                add_tmp_constraints=True,
+            )
 
         # Also test the copy we took earlier.
-        self.randomize_and_check_result(
-            randobj_copy,
-            results,
-            do_tmp_checks,
-            tmp_constraints,
-            tmp_values,
-            tmp_results,
-            post_tmp_results,
-            add_results,
-            add_tmp_constraints=True,
-        )
+        if self.EXPECTED_ERROR_INIT is None:
+            self.randomize_and_check_result(
+                randobj_copy,
+                results,
+                do_tmp_checks,
+                tmp_constraints,
+                tmp_values,
+                tmp_results,
+                post_tmp_results,
+                add_results,
+                add_tmp_constraints=True,
+            )
 
         # Test with seed 1, ensuring results are different
         r1 = random.Random(1)
-        randobj1 = self.get_randobj(r1)
-        if self.EXPECT_FAILURE:
-            self.assertRaises(RandomizationError, randobj1.randomize)
+        if self.EXPECTED_ERROR_INIT is not None:
+            self.assertRaises(self.EXPECTED_ERROR_INIT, self.get_randobj, r1)
         else:
-            results1 = self.randomize_and_time(randobj1, self.iterations)
-            self.check(results1)
-            self.assertNotEqual(results, results1, "Results were the same for two different seeds, check testcase.")
-            if do_tmp_checks:
-                # Check results are also different when applying temporary constraints
-                tmp_results1 = self.randomize_and_time(randobj1, self.iterations, tmp_constraints, tmp_values)
-                self.tmp_check(tmp_results1)
-                self.assertNotEqual(tmp_results, tmp_results1,
-                                    "Results were the same for two different seeds, check testcase.")
+            randobj1 = self.get_randobj(r1)
+            if self.EXPECTED_ERROR_RAND is not None:
+                self.assertRaises(self.EXPECTED_ERROR_RAND, randobj1.randomize)
+            else:
+                results1 = self.randomize_and_time(randobj1, self.iterations)
+                self.check(results1)
+                self.assertNotEqual(results, results1, "Results were the same for two different seeds, check testcase.")
+                if do_tmp_checks:
+                    # Check results are also different when applying temporary constraints
+                    tmp_results1 = self.randomize_and_time(randobj1, self.iterations, tmp_constraints, tmp_values)
+                    self.tmp_check(tmp_results1)
+                    self.assertNotEqual(tmp_results, tmp_results1,
+                                        "Results were the same for two different seeds, check testcase.")
 
         # Test using global seeding, ensuring results are the same
         # Don't add temp constraints this time, so that we can test this object again.
         random.seed(0)
-        randobj0_global = self.get_randobj()
-        self.randomize_and_check_result(
-            randobj0_global,
-            results,
-            do_tmp_checks,
-            tmp_constraints,
-            tmp_values,
-            tmp_results,
-            post_tmp_results,
-            add_results,
-            add_tmp_constraints=False,
-        )
+        if self.EXPECTED_ERROR_INIT is not None:
+            self.assertRaises(self.EXPECTED_ERROR_INIT, self.get_randobj)
+        else:
+            randobj0_global = self.get_randobj()
+            self.randomize_and_check_result(
+                randobj0_global,
+                results,
+                do_tmp_checks,
+                tmp_constraints,
+                tmp_values,
+                tmp_results,
+                post_tmp_results,
+                add_results,
+                add_tmp_constraints=False,
+            )
 
-        # Re-test the the globally-seeded object
-        # Must re-seed the global random module to ensure repeatability.
-        random.seed(0)
-        self.randomize_and_check_result(
-            randobj0_global,
-            results,
-            do_tmp_checks,
-            tmp_constraints,
-            tmp_values,
-            tmp_results,
-            post_tmp_results,
-            add_results,
-            add_tmp_constraints=True,
-        )
+            # Re-test the the globally-seeded object
+            # Must re-seed the global random module to ensure repeatability.
+            random.seed(0)
+            self.randomize_and_check_result(
+                randobj0_global,
+                results,
+                do_tmp_checks,
+                tmp_constraints,
+                tmp_values,
+                tmp_results,
+                post_tmp_results,
+                add_results,
+                add_tmp_constraints=True,
+            )
 
-        # TODO: Fix interaction between global random module and deepcopy.
-        # Details:
-        # There is an issue around copying an object that relies on the
-        # global random object - the state of any copied object is tied to
-        # its original.
-        # Having spent a lot of time debugging this issue, it is still very
-        # difficult to understand.
-        # Each individual copied RandObj instance points to a new random.Random
-        # instance, which shares state with the global module. It appears then
-        # in some instances that the object uses the global value in the random
-        # module, and in others it uses the copied one, meaning the state
-        # diverges.
-        # Right now, all I can conclude is it would take a lot of work to
-        # fully debug it, and it can be worked around by passing objects a
-        # seeded random.Random if the user desires reproducible objects.
+            # TODO: Fix interaction between global random module and deepcopy.
+            # Details:
+            # There is an issue around copying an object that relies on the
+            # global random object - the state of any copied object is tied to
+            # its original.
+            # Having spent a lot of time debugging this issue, it is still very
+            # difficult to understand.
+            # Each individual copied RandObj instance points to a new random.Random
+            # instance, which shares state with the global module. It appears then
+            # in some instances that the object uses the global value in the random
+            # module, and in others it uses the copied one, meaning the state
+            # diverges.
+            # Right now, all I can conclude is it would take a lot of work to
+            # fully debug it, and it can be worked around by passing objects a
+            # seeded random.Random if the user desires reproducible objects.
 
-        # TODO: Make testing of copy more thorough when above issue fixed.
-        # Take a copy, to show that we can. Its behaviour can't be guaranteed
-        # to be deterministic w.r.t. randobj0_global due to issues around
-        # deepcopy interacting with the global random module.
-        # So, just test it can randomize.
-        randobj0_global_copy = deepcopy(randobj0_global)
-        if self.EXPECT_FAILURE:
-            self.assertRaises(RandomizationError, randobj0_global_copy.randomize)
-        else:
-            # Don't check results. Checks may fail due to the interaction
-            # between deepcopy and global random. E.g. if we check that temp
-            # constraints are not followed when not supplied, they may
-            # be due to the interaction between random and deepcopy.
-            # This just ensures it doesn't crash.
-            self.randomize_and_time(randobj0_global_copy, self.iterations)
-            self.randomize_and_time(randobj0_global_copy, self.iterations, tmp_constraints, tmp_values)
+            # TODO: Make testing of copy more thorough when above issue fixed.
+            # Take a copy, to show that we can. Its behaviour can't be guaranteed
+            # to be deterministic w.r.t. randobj0_global due to issues around
+            # deepcopy interacting with the global random module.
+            # So, just test it can randomize.
+            randobj0_global_copy = deepcopy(randobj0_global)
+            if self.EXPECTED_ERROR_RAND is not None:
+                self.assertRaises(self.EXPECTED_ERROR_RAND, randobj0_global_copy.randomize)
+            else:
+                # Don't check results. Checks may fail due to the interaction
+                # between deepcopy and global random. E.g. if we check that temp
+                # constraints are not followed when not supplied, they may
+                # be due to the interaction between random and deepcopy.
+                # This just ensures it doesn't crash.
+                self.randomize_and_time(randobj0_global_copy, self.iterations)
+                self.randomize_and_time(randobj0_global_copy, self.iterations, tmp_constraints, tmp_values)