Enhance conditional mark logic with use_longest for special cases (sonic-net#17317)

What is the motivation for this PR?
In PR sonic-net#16930, we refined the logic of conditional marking to ensure that the longest matching entry with a True condition is prioritized. If the condition for the longest match is False, we then consider the next longest match.

However, there is a special case where the longest match has a False condition, but we still want to adopt this False condition instead of falling back to shorter matches. For example, in a skip scenario:

Shorter matching entries have True conditions, meaning the test should be skipped.
The longest matching entry has a False condition, meaning the test should not be skipped.
In this case, we actually want to respect the longest match and not skip the test.
To address this, this PR introduces a new key, use_longest. When use_longest is set to True, only the conditions of the longest matching entry will be used, ensuring the intended behavior in such edge cases.

How did you do it?
To address this, this PR introduces a new key, use_longest. When use_longest is set to True, only the conditions of the longest matching entry will be used, ensuring the intended behavior in such edge cases.

How did you verify/test it?
Add unit test test cases

Author: yutongzhang-microsoft

tests/common/plugins/conditional_mark/__init__.py

@@ -432,8 +432,17 @@ def find_all_matches(nodeid, conditions, session, dynamic_update_skip_reason, ba
                 match = re.search(condition_entry, nodeid)
             else:
                 match = None
+
+        elif "use_longest" in condition_items.keys():
+            assert isinstance(condition_items["use_longest"], bool), \
+                "The value of 'use_longest' in the mark conditions yaml should be bool type."
+            if nodeid.startswith(condition_entry) and condition_items["use_longest"] is True:
+                all_matches = []
+
+            match = nodeid.startswith(condition_entry)
         else:
             match = nodeid.startswith(condition_entry)
+
         if match:
             all_matches.append(condition)
 
@@ -442,6 +451,9 @@ def find_all_matches(nodeid, conditions, session, dynamic_update_skip_reason, ba
         length = len(case_starting_substring)
         marks = match[case_starting_substring].keys()
         for mark in marks:
+            if mark in ["regex", "use_longest"]:
+                continue
+
             condition_value = evaluate_conditions(dynamic_update_skip_reason, match[case_starting_substring][mark],
                                                   match[case_starting_substring][mark].get('conditions'), basic_facts,
                                                   match[case_starting_substring][mark].get(
@@ -636,7 +648,7 @@ def pytest_collection_modifyitems(session, config, items):
             for match in all_matches:
                 # match is a dict which has only one item, so we use match.values()[0] to get its value.
                 for mark_name, mark_details in list(list(match.values())[0].items()):
-                    if mark_name == "regex":
+                    if mark_name in ["regex", "use_longest"]:
                         continue
                     conditions_logical_operator = mark_details.get('conditions_logical_operator', 'AND').upper()
                     add_mark = False

tests/common/plugins/conditional_mark/unit_test/README.md

@@ -22,7 +22,7 @@ The unit tests cover the following scenarios:
 - Test duplicated conditions
 - Test contradicting conditions
 - Test no matches
-
+- Test only use the longest match
 
 ### How to run tests
 To execute the unit tests, we can follow below command

tests/common/plugins/conditional_mark/unit_test/tests_conditions.yaml

@@ -77,3 +77,49 @@ test_conditional_mark.py::test_mark_7:
     conditions:
       - "topo_type in ['t0']"
       - "topo_type not in ['t0']"
+
+test_conditional_mark.py::test_mark_8:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_8"
+    conditions:
+      - "asic_type in ['mellanox']"
+
+test_conditional_mark.py::test_mark_8_1:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_8_1"
+    conditions:
+      - "asic_type in ['mellanox']"
+
+test_conditional_mark.py::test_mark_8_2:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_8_2"
+    conditions:
+      - "asic_type in ['vs']"
+
+test_conditional_mark.py::test_mark_9:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_9"
+    conditions:
+      - "asic_type in ['vs']"
+
+test_conditional_mark.py::test_mark_9_1:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_9_1"
+    conditions:
+      - "asic_type in ['vs']"
+
+test_conditional_mark.py::test_mark_9_2:
+  use_longest: True
+  skip:
+    reason: "Skip test_conditional_mark.py::test_mark_9_2"
+    conditions:
+      - "asic_type in ['mellanox']"
+  xfail:
+    reason: "Xfail test_conditional_mark.py::test_mark_9_2"
+    conditions:
+      - "asic_type in ['vs']"

tests/common/plugins/conditional_mark/unit_test/unittest_find_all_matches.py

@@ -211,6 +211,87 @@ def test_no_matches(self):
         matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
         self.assertFalse(matches)
 
+    # Test case 11: Test only use the longest match
+    def test_only_use_the_longest_1(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_8"
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+        self.assertFalse(matches)
+
+    def test_only_use_the_longest_2(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_8_1"
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+        self.assertFalse(matches)
+
+    def test_only_use_the_longest_3(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_8_2"
+
+        marks_found = []
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+
+        for match in matches:
+            for mark_name, mark_details in list(list(match.values())[0].items()):
+                marks_found.append(mark_name)
+
+                if mark_name == "skip":
+                    self.assertEqual(mark_details.get("reason"), "Skip test_conditional_mark.py::test_mark_8_2")
+
+        self.assertEqual(len(marks_found), 1)
+        self.assertIn('skip', marks_found)
+
+    def test_only_use_the_longest_4(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_9"
+
+        marks_found = []
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+
+        for match in matches:
+            for mark_name, mark_details in list(list(match.values())[0].items()):
+                marks_found.append(mark_name)
+
+                if mark_name == "skip":
+                    self.assertEqual(mark_details.get("reason"), "Skip test_conditional_mark.py::test_mark_9")
+
+        self.assertEqual(len(marks_found), 1)
+        self.assertIn('skip', marks_found)
+
+    def test_only_use_the_longest_5(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_9_1"
+
+        marks_found = []
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+
+        for match in matches:
+            for mark_name, mark_details in list(list(match.values())[0].items()):
+                marks_found.append(mark_name)
+
+                if mark_name == "skip":
+                    self.assertEqual(mark_details.get("reason"), "Skip test_conditional_mark.py::test_mark_9_1")
+
+        self.assertEqual(len(marks_found), 1)
+        self.assertIn('skip', marks_found)
+
+    def test_only_use_the_longest_6(self):
+        conditions, session_mock = load_test_conditions()
+        nodeid = "test_conditional_mark.py::test_mark_9_2"
+
+        marks_found = []
+        matches = find_all_matches(nodeid, conditions, session_mock, DYNAMIC_UPDATE_SKIP_REASON, CUSTOM_BASIC_FACTS)
+
+        for match in matches:
+            for mark_name, mark_details in list(list(match.values())[0].items()):
+                marks_found.append(mark_name)
+
+                if mark_name == "xfail":
+                    self.assertEqual(mark_details.get("reason"), "Xfail test_conditional_mark.py::test_mark_9_2")
+
+        self.assertEqual(len(marks_found), 1)
+        self.assertIn('xfail', marks_found)
+
 
 if __name__ == "__main__":
     unittest.main()