address review comments in morganstanley#1097

testplan/common/report/base.py

@@ -648,16 +648,15 @@ def pre_order_reports(self):
             elif isinstance(e, Report):
                 yield e
 
-    def disassemble(self):
-        c = []
-        for u in copy.copy(list(self._index.keys())):
-            c.append(self[u])
-            del self[u]
+    def pre_order_disassemble(self):
+        es = copy.copy(self.entries)
+        self.entries.clear()
+        self._index.clear()
 
         yield self
-        for e in c:
+        for e in es:
             if isinstance(e, BaseReportGroup):
-                yield from e.disassemble()
+                yield from e.pre_order_disassemble()
             elif isinstance(e, Report):
                 yield e
 

testplan/runnable/base.py

@@ -140,6 +140,41 @@ def validate_lines(d: dict) -> bool:
     return True
 
 
+def collate_for_merging_parts(es):
+    # to group report entries into buckets, where synthesized ones in the
+    # same bucket containing the previous non-synthesized one
+
+    fst = iter(es)
+    snd = iter(es)
+    _ = next(fst)
+    res = []
+
+    while True:
+        try:
+            snd_e = next(snd)
+            try:
+                fst_e = next(fst)
+            except StopIteration:
+                # snd already at -1, no further (synth) reports
+                res.append((snd_e,))
+                break
+        except StopIteration:
+            break
+
+        grp = [snd_e]
+        while fst_e.category == ReportCategories.SYNTHESIZED:
+            grp.append(fst_e)
+            # there's no prev, cannot set snd at the end
+            _ = next(snd)  # equivalent to ``snd = copy(fst)``
+            try:
+                fst_e = next(fst)
+            except StopIteration:
+                break
+        res.append(tuple(grp))
+
+    return res
+
+
 class TestRunnerConfig(RunnableConfig):
     """
     Configuration object for
@@ -1078,6 +1113,8 @@ def add_post_resource_steps(self):
         self._add_step(self._invoke_exporters)
         self._add_step(self._post_exporters)
         self._add_step(self._stop_remote_services)
+        # FIXME: move _close_file_logger into _post_run_checks, otherwise
+        # FIXME: runner level post run checks output discarded in log file
         self._add_step(self._close_file_logger)
         super(TestRunner, self).add_post_resource_steps()
         self._add_step(self._stop_resource_monitor)
@@ -1261,24 +1298,25 @@ def _merge_reports(
                             raise run
                         else:
                             report.annotate_part_num()
-                            disassembled.append(list(report.disassemble()))
+                            flatten = list(report.pre_order_disassemble())
+                            disassembled.append(
+                                collate_for_merging_parts(flatten)
+                            )
                     else:
                         raise MergeError(
                             f"While merging parts of report `uid`: {uid}, "
                             f"part {report.part[0]} didn't run. Merge of this part was skipped"
                         )
-
-                for it in zip_longest(*disassembled, fillvalue=None):
-                    for e in it:
-                        if e is None:
-                            continue
-                        if not e.parent_uids:
-                            # specially handle mt entry
-                            placeholder_report.non_recursive_merge(e)
-                        else:
-                            placeholder_report.graft_entry(
-                                e, copy(e.parent_uids[1:])
-                            )
+                for it in zip_longest(*disassembled, fillvalue=()):
+                    for t in it:
+                        for e in t:
+                            if not e.parent_uids:
+                                # specially handle mt entry
+                                placeholder_report.non_recursive_merge(e)
+                            else:
+                                placeholder_report.graft_entry(
+                                    e, copy(e.parent_uids[1:])
+                                )
                 placeholder_report.build_index(recursive=True)
                 merged = True
 

tests/functional/testplan/testing/multitest/test_multitest_parts.py

@@ -321,15 +321,14 @@ def test_synthesized_preserved_in_merged():
     # | s1c5  | s1c6  | s1c7  | s1c8  |
     # |       |       | s4s   | s4s   |
     # | s1c9  | s1c10 | s4c1  | s4c2  |
-    # | s4s   |       |       |       |
+    # | s4s   | s4s   |       |       |
     # | s4c3  | s4c4  | s4c5  | s4c6  |
     # | s4c7  | s4c8  | s4c9  | s4c10 |
-    # | s4c11 |       |       |       |
-    # | s4t   | s4t   | s4t   | s4t   |
-
-    # during merging, empty cells will be removed
-    # s4c2 == test_params__arg_0
-    # s4c9 == test_params_2__arg_0
+    # | s4c11 | s4t   | s4t   | s4t   |
+    # | s4t   |       |       |       |
+    # where
+    #   s4c2 == test_params__arg_0
+    #   s4c9 == test_params_2__arg_0
 
     s4 = plan.report["mtest"]["Suite4"]
     assert (
@@ -363,6 +362,69 @@ def test_synthesized_preserved_in_merged():
     es = plan.report["mtest"]["Environment Stop"]
     assert len(es) == 4
     assert es.entries[0].uid == "Before Stop - part(2/4)"
-    assert es.entries[1].uid == "Before Stop - part(0/4)"
-    assert es.entries[2].uid == "Stopping - part(2/4)"
+    assert es.entries[1].uid == "Stopping - part(2/4)"
+    assert es.entries[2].uid == "Before Stop - part(0/4)"
     assert es.entries[3].uid == "Stopping - part(0/4)"
+
+
+@testsuite
+class Suite5:
+    def teardown(self, env, result):
+        result.log("padding")
+
+    @testcase
+    def c1(self, env, result):
+        result.true(True)
+
+
+@testsuite
+class Suite6:
+    @testcase
+    def c1(self, env, result):
+        result.false(True)
+
+    @testcase
+    def c2(self, env, result):
+        result.true(False)
+
+
+@testsuite
+class Suite7:
+    @testcase
+    def c1(self, env, result):
+        result.false(False)
+
+    @testcase
+    def c2(self, env, result):
+        result.false(False)
+
+    @testcase
+    def c3(self, env, result):
+        result.false(False)
+
+
+def test_order_maintained_w_synthesized_in_merged():
+    plan = TestplanMock(name="plan", merge_scheduled_parts=True)
+    for i in range(3):
+        plan.add(
+            MultiTest(
+                "mtest",
+                [Suite5(), Suite6(), Suite7()],
+                part=(i, 3),
+            )
+        )
+
+    assert plan.run().run is True
+
+    # | p0    | p1    | p2    |
+    # | ----- | ----- | ----- |
+    # | s5c1  | s6c1  | sbc2  |
+    # | s5t   |       |       |
+    # | s7c1  | s7c2  | s7c3  |
+
+    # round-robin without collation will result in order [s7c2, s7c3, s7c1]
+
+    s7 = plan.report["mtest"]["Suite7"]
+    assert s7.entries[0].uid == "c1"
+    assert s7.entries[1].uid == "c2"
+    assert s7.entries[2].uid == "c3"

tests/unit/testplan/common/report/test_base.py

@@ -369,9 +369,9 @@ def test_graft_round_trip(self):
         parent.append(child)
 
         refs = list(grand_parent.pre_order_reports())
-        parts = list(grand_parent.disassemble())
+        parts = list(grand_parent.pre_order_disassemble())
 
-        # disasembled in place
+        # disassembled in place
         assert not grand_parent.entries
         assert all(map(lambda x, y: x is y, refs, parts))