Skip to content

fix: HTML report merge crash and harden result file handling - #158

Open
mkzung wants to merge 1 commit into
NethermindEth:mainfrom
mkzung:fix/report-merge-crash
Open

fix: HTML report merge crash and harden result file handling#158
mkzung wants to merge 1 commit into
NethermindEth:mainfrom
mkzung:fix/report-merge-crash

Conversation

@mkzung

@mkzung mkzung commented Jul 2, 2026

Copy link
Copy Markdown

Four fixes in the result-merging / report-generation path, plus the repo's first tests/test_utils.py.

  1. merge_html crashed, then produced broken HTML (utils.py). It searched for a <thread> tag (not valid HTML; the tag is <thead>) and indexed [0], so find_all('thread')[0] raised IndexError and crashed the merge. Beyond the crash, the row merge appended each second-table <tr> into the first table's <tr>, nesting rows and putting a data row inside <thead>. Since the merged tables are the same client's results from two report folders (each client has a unique id="table_<client>"), the intent is to stack rows under one header. This removes the duplicate <thead> (decomposing every match, so it also handles a table with none) and appends the second table's body rows into the first table's <tbody>, producing valid, correctly-stacked HTML.
  2. merge-results.py. Replaced os.system(f'cp {src} {dst}') with shutil.copy, so paths with spaces or shell metacharacters are handled correctly.
  3. report_html.py tags. Fixed a malformed <pre"> tag (should be <pre>).
  4. report_html.py table close order. The report emitted </table> before </tbody>; swapped them to close </tbody> then </table>.

Adds tests/test_utils.py (matching the naming of the existing suite) asserting the merged output has one header, all rows stacked in the body, and no nested <tr>, plus the no-<thead> case; a root conftest.py makes the top-level modules import under pytest. Run with python -m pytest tests/test_utils.py -v. Verified locally that the merged output renders as valid, correctly-stacked HTML.

@mkzung
mkzung marked this pull request as ready for review July 2, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant