Skip to content

Improve PDF test report generation and typography#59

Merged
rical merged 17 commits intorical:masterfrom
kernelkit:test-report-changes
Sep 15, 2025
Merged

Improve PDF test report generation and typography#59
rical merged 17 commits intorical:masterfrom
kernelkit:test-report-changes

Conversation

@troglobit
Copy link
Contributor

This PR improves the 9pm test framework's PDF report generation with better layout, and user experience for print:

  • Combined test specifications, results, and output in unified sections. This eliminates the problematic cross-references and improve page flow for printed documents
  • Added cover page with optional logo and executive summary for key stakeholders
  • Added summary statistics: PASS/FAIL/SKIP counts
  • Adjusted page breaks and layout preventing orphaned content
  • Each test now shows its result status prominently alongside specifications

@rical
Copy link
Owner

rical commented Sep 8, 2025

Looks really nice!

We should honor the test numbers and test names in the report. Names are especially valuable as they can be used in conjunction with options that makes the same code do different things.

0013-test-report-creation
✅ : 0014-test.pl
✅ : 0015-static-named
✅ : 0016-dynamic.yaml
✅ : 0017-bash.sh
✅ : 0018-dynamic-named

Current Codebase:
Screenshot from 2025-09-08 09-44-21

This PR:
Screenshot from 2025-09-08 09-45-38

rical and others added 3 commits September 8, 2025 11:21
Signed-off-by: Richard Alpe <richard@bit42.se>
Relocate testing and doctoring to a Makeflle for devs. to use, and let
the GitHub workflow use that instead.

Note: the 'make' utility is a dependency of build-essential so we can
      safely call it from the workflow.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Change disposition of the report from separate output and specification
sections, to a combined specification, result and output for each test.

This greatly improves the accessibility for the generated PDF, keeping
all relevant information in one place, and eliminating the unintuitive
link to the Test Specification from the output.  Links are problematic
in printed media.

Each test's heading now includes colored PASS/FAIL/SKIP which lines up
nicely in the generated Table of Contents, similar in fashion to how
write_report_result_tree() already works.  Several unicode symbols were
tested instead of the spelled-out result, e.g., ☑ but the result was
very difficult to read.

The call to write_report_result_tree() is temporarily dropped in this
commit, only to reintroduce it in the next in a new Test Info section
under the Test Summary.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
Copy link
Contributor Author

Changes since post AFK review and discussions last week:

  • Factored out logic from GitHub workflow to a Makefile to ease developer workflows as well
  • Change name: to allow spaces and, basically, any characters for test title
  • Ensure filenames derived from name: are sane using a local version of slugify()
  • Include relative path to test, in addition to name:, in 9pm standard output. May be difficult for a developer to find the faulty test otherwise
  • Include relative path to test with arguments in test log
  • Sometimes .git is a file, when 9pm is a submodule
  • ... and minor syntax/warning fixes
name

We also discussed using a ☑️ in the test title, for nice summaries in the table of contents. However, due to varying support these unicode characters in popular fonts, and poor readability, this was dropped in favor of spelling-out the result: PASS, FAIL, SKIP (same length!).

The masked fail and masked skip have been implemented as crossed-out FAIL and SKIP. This keeps their length the same as the "normal" test result codes, and possibly helps the user understand what's going on. Please see the attached report.pdf for an example.

@troglobit
Copy link
Contributor Author

troglobit commented Sep 14, 2025

Weird error in self tests on last run, I don't get it locally when running the new make check, so I've extended the error message from self_test/run.py slightly, hopefully we get some hint this time.

Update: apparently import yaml failed in the "Simplify developer workflow" commit, so I've reverted parts of that, only kept the README update. But it's very odd behavior, maybe the culprit is in actions/setup-python@v3? Because Debian/Ubuntu have a working python3 so why would that action be needed?

kernelkit/9pm workflow passing: https://github.com/kernelkit/9pm/actions/runs/17707782707/job/50322184569

@troglobit
Copy link
Contributor Author

Yeah, it has to be actions/setup-python@v3 that breaks the debian/ubuntu python. It seems it installs a separate Python in /opt/hostedtoolcache/Python/$VERSION/x64 and modifies the PATH so that python3 now points to the "Actions Python" instead of the system Python.

I've reverted back to the original setup, only keeping the simplified setup instructions in the README. I hope that's OK?

The new Test Overview sub-section shows the number of tests to have
passed, failed, and have been skipped, along with the full details
from write_report_result_tree().

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
The cover page provide a professional document header with an optional
logo, title, and key artifact details: name, version and dete of test.
Overall improving report presentation for all stakeholders.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Allow test output on same page.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Adjust size of heading slightly so it is evident that even a h4 is
distinguishable from bold text.

Drop underline for h3 since it does not play well with colored text.
The line under a colored word also becomes colored.

For links, which are removed in this patch series, let's use a clear
blue color so that users recognize them as links.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This commits opens up 'name:' as a free-text field to use as the test's
"title" in report generation and general 9pm output.  When omitted the
fallback is the basename of the new unix_name, as before.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
- Show various uses for the name: field
- Fix Bash test spec: Python -> Bash

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
With the new liberal free-form 'name:' we must take better care to
ensure all filenames are normalized.  To keep the dependencies low
we use a local good-enough slugify() for this purpose.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Use raw strings for regex patterns to avoid Python warnings about
invalid escape sequences like \d and \w.

Fixed warnings like:
  SyntaxWarning: invalid escape sequence '\d'
  plan = re.search('^(\d+)..(\d+)', string)

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
When working on 9PM while it is used as a GIT submodule in another
project, .git is not a directory.  Handle this case by looking up
the actual .git directory:

    $ ls -la .git
    -rw-rw-r-- 1 bob bob 31 dec  1  2023 .git
    $ cat .git
    gitdir: ../../.git/modules/9pm

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
All dependencies are actually available from apt sources, so it's possible
to skip both pip and gem.  (For some reason this does however not work in
GitHub actions.)

Supplement instructions in README and mention shortcut on Debian/Ubuntu.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Add helpful relative path to test output for test developers in addition
to the human (mgmt) friendly text.

Also, use f-string for "Skip test" => shorter and mroe readable code.

    Running suite Suite Skip

    Starting test suite_skip.tcl (unit_tests/harness/suite_skip.tcl)
    2025-09-14 06:29:47 1..1
    2025-09-14 06:29:47 ok 1 # skip suite Required argument "require-value-foo" != "foo"

    Starting test suite_skip.tcl (unit_tests/harness/suite_skip.tcl)
    Skip test suite_skip.tcl (suite skip)
    Test skip is masked in suite

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
With the new free-form 'name:' it can be difficult for a test developer
reading the log output to immediately make out which test (file) they
are looking at.  This commit adds the relative path to the test to the
log and formats any arguments using the textwrap package.

Example:

    Starting: path/to/test suite-supplied mode:long scratch:/tmp/9pm_i78hxc2f
                           base:/home/jocke/src/infix/test/9pm/unit_tests/harness
                           optional-positive:yes skip-positive:yes cmdl-supplied
    2025-09-14 07:03:39 1..6
    ...

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Copy link
Owner

@rical rical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work! The new report looks amazing and the ambiguous prettified naming is a great improvement.

Basically the only remark I have is that we should print test numbers when starting tests and in the result tree. Perhaps it should also be included in the report as "metadata".

Old behavior:

Starting test 0051-plan-last
....
            |-- o 0051-plan-last

New

Starting test Plan Last (unit_tests/lib_tcl/tap/plan.sh) 
...
            |-- o Plan Last  

Suggestion

Starting test 0051 Plan Last (unit_tests/lib_tcl/tap/plan.sh) 
...
            |-- o 0051 Plan Last  

onfail = {}
onfail['case'] = os.path.join(dirname, test['onfail'])
onfail['unix_name'] = 'onfail'
onfail['name'] = 'onfail'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can improve this name now that name is more loosely tied to the inner mechanics?


def write_report_output(file, data, depth):
for test in data['suite']:
def resultfmt(test):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function name is a bit to generic. The main problem here isn't really your naming, but the lack of scoping / classing. Ugly as it might be I would suggest including report in the name for now. Then we can refactor this once we break report stuff out into its own world.


if not os.path.isdir(gitdir):
vcprint(pcolor.orange, f"warning, no .git dir in path ({path})")
if os.path.isfile(git_path):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to have a comment describing why we need this logic (file | dir)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@rical rical merged commit 8f5294b into rical:master Sep 15, 2025
1 check passed
@troglobit
Copy link
Contributor Author

Fantastic work! The new report looks amazing and the ambiguous prettified naming is a great improvement.

Thanks, very easy to work with this code base!

Basically the only remark I have is that we should print test numbers when starting tests and in the result tree. Perhaps it should also be included in the report as "metadata".
[snip]
Starting test 0051 Plan Last (unit_tests/lib_tcl/tap/plan.sh)
...
|-- o 0051 Plan Last

Yup, that would've been my idea as well. I have some initial prototype work if you're interested?

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.

2 participants