Skip to content

Commit c870fe5

Browse files
pluskymimi1vx
plusky
authored and
mimi1vx
committed
fix(export): make re-exports idempotent and stop over-deletion
Four related defects, all in how the export pipeline mutates the template on a second run: - installlogs_lines() inserted a fresh 'Links for update logs:' header on every call; individual links were de-duplicated but the header was not, so manual/kernel re-exports stacked empty duplicate sections. An existing header is now reused, new links are appended after the section's links, the trailing blank is only added when missing, and empty duplicate headers stacked by pre-fix exports are dropped so a damaged template converges (link-bearing duplicates are never touched). A hand-trimmed section (header directly followed by the footer) gets its canonical blank restored so links cannot land after the footer. - BaseExport.install_results() inserted the 'All installation tests done in openQA' notice unconditionally; on kernel re-export the old and new copies are separated by the inserted blank line, so the notice multiplied with each run. It is now inserted only when absent, and copies stacked by pre-fix exports are reduced back to one. - AutoExport.install_results() bounded its section replacement with list.index('## export MTUI:'), which requires the whole line to equal the marker -- the real footer is '## export MTUI:<version> ...', so the fallback always raised and end ran to len(template): the replacement deleted the footer and anything appended after the install-tests section. The footer is now located with a substring scan, like the two other places that test for it. - ManualExport.install_results()'s stale-result cleanup was dead: the host-tracking regex required two spaces after 'reference host:' (the template emits one) and read group(0), so no stale line was ever removed. The pattern now captures the hostname, the host header line is kept, and -- adversarial-review catch on the revived cleanup -- c_host resets at the host block's 'comment:' boundary (the same convention the verdict loop uses), so the deletion window cannot bleed past the last host section and eat tester-authored notes like 'reproducer : FAILED before update' from the regression-tests section. Each fix and each review amendment is revert-verified by tests/test_export_idempotency.py (10 tests).
1 parent 1f39e71 commit c870fe5

5 files changed

Lines changed: 495 additions & 19 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,26 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht
318318
on hosts whose `/etc/os-release` single-quotes its values (permitted by the
319319
os-release spec). The parser's character classes matched a double quote or
320320
a literal pipe — `["|]` — instead of double-or-single quote.
321+
- Re-exporting a testreport no longer degrades the template. Manual and
322+
kernel re-exports used to stack an empty duplicate `Links for update
323+
logs:` header per run (the links were de-duplicated, the header was not) —
324+
new links now go under the existing section, and empty duplicate headers
325+
stacked by earlier exports are cleaned up so damaged templates converge.
326+
The kernel re-export also duplicated the "All installation tests done in
327+
openQA" notice on every run; it is now inserted only once and stacked
328+
copies are reduced back to one.
329+
- An auto-workflow `export` on a template without a `Links for update logs:`
330+
section no longer deletes everything from the install-tests section to the
331+
end of the file (export footer and any appended notes included). The
332+
fallback boundary looked for a line exactly equal to `## export MTUI:`,
333+
which never matches the real footer; it now scans for the footer as a
334+
substring.
335+
- Re-exporting a manual testreport now actually refreshes stale per-command
336+
result lines (`… : SUCCEEDED`/`FAILED`/`INTERNAL ERROR`) for hosts in the
337+
current session. The host-tracking pattern required two spaces after
338+
`reference host:` (the template emits one) and compared the whole match
339+
instead of the hostname, so the cleanup never removed anything; other
340+
hosts' sections remain untouched.
321341
- `mtui-mcp` no longer corrupts a `commit`/`lock` call that also carries a
322342
`template` argument, nor a backgrounded `run` that fans out across several
323343
loaded templates. A `-m`/`-c` message or a `run` command line no longer

‎mtui/update_workflow/export/auto.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,20 @@ def install_results(self) -> None:
6666
while end > start and self.template[end - 1] == "\n":
6767
end -= 1
6868
except ValueError:
69-
try:
70-
end = self.template.index("## export MTUI:", start)
71-
except ValueError:
72-
end = len(self.template)
69+
# Substring scan for the footer: the actual line is
70+
# '## export MTUI:<version>, ... by <user>\n', never exactly
71+
# '## export MTUI:', so list.index() could not match it -- end
72+
# ran to len(template) and the replacement deleted everything
73+
# after the Install-tests section (footer included, plus any
74+
# free-text notes a tester had appended).
75+
end = next(
76+
(
77+
i
78+
for i in range(max(start, 0), len(self.template))
79+
if "## export MTUI:" in self.template[i]
80+
),
81+
len(self.template),
82+
)
7383

7484
block = [
7585
"##############\n",

‎mtui/update_workflow/export/base.py‎

Lines changed: 86 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,38 @@ def installlogs_lines(self, filenames) -> None:
9898
o = i + 1
9999
break
100100

101-
index = len(self.template)
102-
if "## export MTUI:" in self.template[-1]:
103-
index -= 1
104-
self.template.insert(index, "\n")
105-
self.template.insert(index + 1, "Links for update logs:\n")
106-
self.template.insert(index + 2, "\n")
107-
index += 2
101+
marker = "Links for update logs:\n"
102+
try:
103+
# Reuse an existing section: manual/kernel exports run this on
104+
# every export, and unconditionally inserting a fresh header
105+
# stacked empty 'Links for update logs:' sections that grew
106+
# with each re-export (the links themselves were de-duplicated,
107+
# the header was not). New links are appended after the
108+
# section's existing links.
109+
index = self.template.index(marker) + 1
110+
self._drop_empty_link_sections(marker, index)
111+
if index >= len(self.template) or (
112+
self.template[index] != "\n"
113+
and str(self.config.reports_url) not in self.template[index]
114+
):
115+
# Hand-trimmed template: the header is followed directly by
116+
# foreign content (e.g. the export footer). Restore the
117+
# canonical blank so the links land inside the section, not
118+
# after the footer.
119+
self.template.insert(index, "\n")
120+
while (
121+
index + 1 < len(self.template)
122+
and str(self.config.reports_url) in self.template[index + 1]
123+
):
124+
index += 1
125+
except ValueError:
126+
index = len(self.template)
127+
if "## export MTUI:" in self.template[-1]:
128+
index -= 1
129+
self.template.insert(index, "\n")
130+
self.template.insert(index + 1, marker)
131+
self.template.insert(index + 2, "\n")
132+
index += 2
108133

109134
add_empty_line = False
110135
for fn in filenames:
@@ -114,9 +139,48 @@ def installlogs_lines(self, filenames) -> None:
114139
self.template.insert(index, install_log)
115140
add_empty_line = True
116141

117-
if add_empty_line:
142+
if add_empty_line and (
143+
index + 1 >= len(self.template) or self.template[index + 1] != "\n"
144+
):
118145
self.template.insert(index + 1, "\n")
119146

147+
def _drop_empty_link_sections(self, marker: str, search_from: int) -> None:
148+
"""Remove empty duplicate 'Links for update logs:' headers.
149+
150+
Pre-fix exports stacked one fresh header per run while the links
151+
stayed under the original section, so damaged templates carry
152+
trailing header blocks with nothing but blanks under them. They
153+
would otherwise survive forever (``dedup_lines`` never collapses
154+
blank-separated duplicates). A duplicate section that does hold
155+
links is left alone -- never delete content.
156+
157+
Args:
158+
marker: The section header line.
159+
search_from: Index to start scanning at (just past the first,
160+
kept, header).
161+
162+
"""
163+
i = search_from
164+
while True:
165+
try:
166+
j = self.template.index(marker, i)
167+
except ValueError:
168+
return
169+
k = j + 1
170+
while k < len(self.template) and self.template[k] == "\n":
171+
k += 1
172+
if (
173+
k < len(self.template)
174+
and str(self.config.reports_url) in (self.template[k])
175+
):
176+
i = k # a section with real links: keep it
177+
continue
178+
# Empty duplicate: drop the header, its trailing blanks, and the
179+
# framing blank the old code inserted before it.
180+
start = j - 1 if j > 0 and self.template[j - 1] == "\n" else j
181+
del self.template[start:k]
182+
i = start
183+
120184
def dedup_lines(self) -> None:
121185
"""Deduplicates lines in the template."""
122186
pr_line = None
@@ -214,8 +278,17 @@ def inject_openqa(self) -> None:
214278
def install_results(self) -> None:
215279
"""Adds installation results to the template."""
216280
index = self.template.index("Test results by product-arch:\n", 0)
217-
self.template.insert(
218-
index + 3,
219-
"All installation tests done in openQA please see installlogs section\n",
220-
)
221-
self.template.insert(index + 4, "\n")
281+
line = "All installation tests done in openQA please see installlogs section\n"
282+
# Idempotent: on a kernel re-export the previous run's notice is
283+
# still there, separated from a fresh insert by the blank line, so
284+
# dedup_lines() never collapsed them and the notice multiplied with
285+
# every export. Copies stacked by pre-fix exports are dropped so a
286+
# damaged template converges back to a single notice.
287+
while self.template.count(line) > 1:
288+
extra = len(self.template) - 1 - self.template[::-1].index(line)
289+
del self.template[extra]
290+
if extra < len(self.template) and self.template[extra] == "\n":
291+
del self.template[extra]
292+
if line not in self.template:
293+
self.template.insert(index + 3, line)
294+
self.template.insert(index + 4, "\n")

‎mtui/update_workflow/export/manual.py‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,26 @@ def install_results(self) -> None:
240240
c_host = None
241241
tmp_template = []
242242
for line in self.template:
243-
match = re.search(r"reference host:\s (.*)$", line)
243+
# Track which host section we are in so only the *current
244+
# session's* hosts get their stale result lines refreshed. The
245+
# old pattern required two spaces after the colon (the template
246+
# emits one) and read group(0) (the whole match, never a bare
247+
# hostname), so no stale line was ever removed. The host line
248+
# itself is kept -- it is the section header.
249+
match = re.search(r"reference host:\s+([^)\s]+)", line)
244250
if match:
245-
c_host = match.group(0)
251+
c_host = match.group(1)
252+
tmp_template.append(line)
253+
continue
254+
255+
if c_host is not None and line.startswith("comment:"):
256+
# End of this host's block (same boundary convention as the
257+
# verdict loop above). Without the reset the deletion window
258+
# bled past the last host section and ate tester-authored
259+
# lines like 'reproducer : FAILED before update' from the
260+
# regression-tests notes.
261+
tmp_template.append(line)
262+
c_host = None
246263
continue
247264

248265
if (

0 commit comments

Comments
 (0)