Add Markdown sim report generation & output to CLI #110
Merged
machshev merged 6 commits intolowRISC:masterfrom Mar 10, 2026
Merged
Add Markdown sim report generation & output to CLI #110machshev merged 6 commits intolowRISC:masterfrom
machshev merged 6 commits intolowRISC:masterfrom
Conversation
AlexJones0
force-pushed
the
sim_markdown_cli_report
branch
7 times, most recently
from
a117ddc to
ce0abea
Compare
machshev
reviewed
Mar 10, 2026
| """Results per test stage.""" | ||
| coverage: CoverageMetrics | None | ||
| """Coverage metrics.""" | ||
| cov_report_page: Path | None |
Collaborator
There was a problem hiding this comment.
Not sure what this looks like exactly, however in general I'd prefer to have deterministic paths (i.e coverage report page is always in this relative path) rather than having an extra field. We can then calculate the field at runtime... potentially provide it as an @property instead if possible?
| name: str | ||
| """Name of the job.""" | ||
|
|
||
| qual_name: str |
Collaborator
There was a problem hiding this comment.
All the different types of names is something I've been looking at in general and thinking it's a bit messy and opaque. It would be nice if we could instead capture the component parts instead, so it's clearer the difference between the 2-3 different name types.
Maybe provide @property methods for backward compatibility?
Not quite sure what this should look like really? This isn't for this PR either, just a note for ideas.
machshev
reviewed
Mar 10, 2026
| line: int | None | ||
| """Line number within the log if there is one.""" | ||
|
|
||
| log_path: Path | None |
Collaborator
There was a problem hiding this comment.
Again, can this be calculated, or do we have to store it?
AlexJones0
force-pushed
the
sim_markdown_cli_report
branch
from
ce0abea to
5f3028d
Compare
This information will be needed for the Markdown CLI report, and might also be worth surfacing in the HTML report as well. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This information needs to be surfaced in the Markdown CLI report, and might also be worth presenting in the HTML report later. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This information will be needed for reporting in the Markdown CLI logs. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Reproduce the behaviour of the original DVSim within the new reporting abstractions by introducing a `MarkdownReportRenderer` (stubbed for now, to be filled out in future commits) and relevant display logic - with per-block results shown as INFO logs, and summary results shown as a print (if the summary is a primary config file). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Generally intended to resemble the original reports that were removed from DVSim as closely as possible. Note the TODOs - currently the progress table is missing because this information is not being computed nor captured correctly here. Also the stage/overall totals in the result table are not correct, but this is because the SimCfg calculations are not correct, instead of the Markdown presentation logic being incorrect. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
If a link to a directory containing an output HTML report is provided,
then the generated table will use Markdown links to the generated
reports. This is using different logic to the original DVSim in
OpenTitan, because that did not make sense, for two reasons:
1. Reports were relative to the generated Markdown file, but we no
longer write a Markdown report to disk.
2. The report structure as a whole has changed.
Instead, while we still provide the ability to customise the relative
path to report paths, if unspecified (which it is currently), we just
print CLI report links relative to the CWD.
It might be the case that this doesn't have much value in current DVSim
and so should just be removed altogether.
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
sim_markdown_cli_report
branch
from
5f3028d to
8279cb0
Compare
machshev
reviewed
Mar 10, 2026
|
|
||
| return {"report.md": report} | ||
|
|
||
| def render_block(self, results: SimFlowResults) -> ReportArtifacts: |
Collaborator
There was a problem hiding this comment.
Might be nice if in the future this becomes a jinja2 template? Although not sure if it will actually look cleaner or not?
machshev
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is the fourth of a series of 4 PRs which aim to fix issues in HTML report generation and add back Markdown CLI reports to DVSim. As such, it depends on the previous PR #109 being merged. The first 3 commits in this PR come from there; only the last 6 commits are relevant for reviewing this PR.
Closes #75 (mostly, an issue should be made for progress tables as described below).
This PR re-introduces the Markdown report generation logic of the original DVSim and outputs it on the command-line. Logic is introduced to ensure this matches the original report, with a couple of noted exceptions:
--map-full-testplanis not added yet because this was originally computed in theTestplan, and is not computed in the current results objects. There probably needs to a be a bit of an overhaul in how results are stored to handle this (and to handle the miscounting of total test results per stage/overall), but that is more complex and so is not done in this PR.Testplan, since we no longer go through theTestplanitself to generate results. I want to get rid of these stale functions, but they are currently used by thetestplanner.pyscript, which is currently broken by other changes. We should probably refactor the result handling to be able to support thetestplanner.pyuse case (and fix existing errors), and then re-write that small script, so we can remove these duplicated functions.It is recommended to review commit-by-commit, checking the commit messages for more info.