[dvsim] Address #3071 comments

Plus, som minor refactoring.

Signed-off-by: Srikrishna Iyer <sriyer@google.com>
diff --git a/util/dvsim/Modes.py b/util/dvsim/Modes.py
index 4e2f979..c49d6cb 100644
--- a/util/dvsim/Modes.py
+++ b/util/dvsim/Modes.py
@@ -77,19 +77,60 @@
         if not mode.name == self.name and not is_sub_mode:
             return False
 
-        # only merge the lists; if strs are different, then throw an error
-        attrs = self.__dict__.keys()
-        for attr in attrs:
-            # merge lists together
-            self_attr_val = getattr(self, attr)
-            mode_attr_val = getattr(mode, attr)
+        # Merge attributes in self with attributes in mode arg, since they are
+        # the same mode but set in separate files, or a sub-mode.
+        for attr, self_attr_val in self.__dict__.items():
+            mode_attr_val = getattr(mode, attr, None)
 
-            if type(self_attr_val) is list and type(mode_attr_val) is list:
+            # If sub-mode, skip the name fields - they could differ.
+            if is_sub_mode and attr in ['name', 'mname']:
+                continue
+
+            # If mode's value is None, then nothing to do here.
+            if mode_attr_val is None:
+                continue
+
+            # If self value is None, then replace with mode's value.
+            if self_attr_val is None:
+                setattr(self, attr, mode_attr_val)
+                continue
+
+            # If they are equal, then nothing to do here.
+            if self_attr_val == mode_attr_val:
+                continue
+
+            # Extend if they are both lists.
+            if isinstance(self_attr_val, list):
+                assert isinstance(mode_attr_val, list)
                 self_attr_val.extend(mode_attr_val)
-                setattr(self, attr, self_attr_val)
+                continue
 
-            elif not is_sub_mode or attr not in ["name", "mname"]:
-                self.check_conflict(mode.name, attr, mode_attr_val)
+            # If the current val is default, replace with new.
+            scalar_types = {str: "", int: -1}
+            default_val = scalar_types.get(type(self_attr_val))
+
+            if type(self_attr_val) in scalar_types.keys(
+            ) and self_attr_val == default_val:
+                setattr(self, attr, mode_attr_val)
+                continue
+
+            # Check if their types are compatible.
+            if type(self_attr_val) != type(mode_attr_val):
+                log.error(
+                    "Mode %s cannot be merged into %s due to a conflict "
+                    "(type mismatch): %s: {%s(%s), %s(%s)}", name, self.name,
+                    attr, str(self_attr_val), str(type(self_attr_val)),
+                    str(mode_attr_val), str(type(mode_attr_val)))
+                sys.exit(1)
+
+            # Check if they are different non-default values.
+            if self_attr_val != default_val and mode_attr_val != default_val:
+                log.error(
+                    "Mode %s cannot be merged into %s due to a conflict "
+                    "(unable to pick one from different values): "
+                    "%s: {%s, %s}", name, self.name, attr, str(self_attr_val),
+                    str(mode_attr_val))
+                sys.exit(1)
 
         # Check newly appended sub_modes, remove 'self' and duplicates
         sub_modes = self.get_sub_modes()
@@ -102,30 +143,6 @@
             self.set_sub_modes(new_sub_modes)
         return True
 
-    def check_conflict(self, name, attr, mode_attr_val):
-        self_attr_val = getattr(self, attr)
-        if self_attr_val == mode_attr_val:
-            return
-
-        if mode_attr_val is None:
-            # No override here
-            return
-
-        default_val = None
-        if type(self_attr_val) is int:
-            default_val = -1
-        elif type(self_attr_val) is str:
-            default_val = ""
-
-        if self_attr_val != default_val and mode_attr_val != default_val:
-            log.error(
-                "mode %s cannot be merged into %s due to conflicting %s {%s, %s}",
-                name, self.name, attr, str(self_attr_val), str(mode_attr_val))
-            sys.exit(1)
-        elif self_attr_val == default_val:
-            self_attr_val = mode_attr_val
-            setattr(self, attr, self_attr_val)
-
     @staticmethod
     def create_modes(ModeType, mdicts):
         '''
@@ -422,9 +439,18 @@
         self.type = ""
         if not hasattr(self, "mname"):
             self.mname = "regression"
+
+        # The `tests` member is typically a list, but it defaults to None.
+        # There are 3 possible cases after all the HJson files are parsed, when
+        # this particular regression is supplied to be run:
+        #
+        # 1. `tests` == None:   This is treated as "run ALL available tests".
+        # 2. `tests` == []:     No available tests to run
+        # 3. `len(tests)` > 0:  The provided set of tests are run.
         self.tests = None
-        self.reseed = None
         self.test_names = []
+
+        self.reseed = None
         self.excl_tests = []  # TODO: add support for this
         self.en_sim_modes = []
         self.en_run_modes = []
@@ -523,6 +549,8 @@
                 regression_obj.run_opts.extend(run_mode_obj.run_opts)
 
             # Unpack tests
+            # If `tests` member resolves to None, then we add ALL available
+            # tests for running the regression.
             if regression_obj.tests is None:
                 log.log(VERBOSE,
                         "Unpacking all tests in scope for regression \"%s\"",