Skip to content

Keep HTML reporter registered when MTA is unavailable#5

Open
artemry-nv wants to merge 1 commit into
masterfrom
fix/html-reporter-no-mta
Open

Keep HTML reporter registered when MTA is unavailable#5
artemry-nv wants to merge 1 commit into
masterfrom
fix/html-reporter-no-mta

Conversation

@artemry-nv

@artemry-nv artemry-nv commented Jun 22, 2026

Copy link
Copy Markdown

Summary

On external clusters without a local MTA (e.g. Pre-Tyche), MTT::Mail::Init fails and the HTML reporter previously returned 0 at Init, so it was never registered and Submit/Finalize never wrote any HTML reports. This disables email delivery (sets $to=undef) but returns 1 so HTML reports are still generated on disk.

Email-sending in Submit is already guarded by if ($to), so clearing $to cleanly skips email while HTML table generation continues.

Test plan

  • Container run on Pre-Tyche generates All_phase-summary.html and per-suite HTML on disk without an MTA
  • Clusters with an MTA still send email reports (unchanged path)

Reported-by: Bartlomiej Pawulski bpawulski@nvidia.com

On external clusters without a local MTA (e.g. Pre-Tyche), MTT::Mail::Init
fails and the HTML reporter returned 0 at Init, so it was never registered
and Submit/Finalize never wrote any HTML reports. Disable email delivery
(set $to=undef) but return 1 so HTML reports are still generated on disk.

Reported-by: Bartlomiej Pawulski <bpawulski@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

In MTT::Reporter::HTMLReport::Init, the error path triggered by a failed MTT::Mail::Init call now sets $to to undef, logs that email delivery is disabled, and returns 1 (success) instead of 0 (failure), allowing HTML report files to still be written to disk.

Changes

HTML Reporter Mail-Failure Fallback

Layer / File(s) Summary
HTMLReport::Init mail-failure fallback
lib/MTT/Reporter/HTMLReport.pm
When MTT::Mail::Init fails, $to is set to undef and the function now returns 1 instead of 0, keeping the HTML reporter registered and able to write reports while disabling email delivery.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 When the mail server frowns and won't play,
The rabbit still hops on its way —
No email? No fuss!
HTML's a plus,
The report shall be written today! ✉️📄

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly matches the main change: modifying HTML reporter initialization to succeed when MTA is unavailable, allowing registration and HTML output to disk.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/html-reporter-no-mta

Comment @coderabbitai help to get the list of available commands.

@artemry-nv artemry-nv requested a review from bpawulski June 22, 2026 20:43
@artemry-nv

Copy link
Copy Markdown
Author

Validated on Pre-Tyche (SLURM job 2273616, ucx-enroot suite, aarch64 container).

With this fix, the HTML reporter stays registered despite no MTA and writes reports to disk via save_intermediate_report=1:

  • All_phase-summary.html (index, shows "Enroot with Pyxis" + /opt/hpcx + image hpcx-v2.50-...-aarch64-latest-nightly)
  • Test_Run-osu_micro_benchmark_one_sided-...html (611 KB, 647 tests passed)
  • MPI_Install-...html, Test_Build-...html, etc.

The prior unpatched run (job 2273422) produced zero HTML files under the same configuration, confirming this is the fix.

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