Skip to content

Add returnStats and statsFile options to TestDirectory - #6581

Draft
limakzi wants to merge 1 commit into
masterfrom
testdirectory-stats
Draft

limakzi wants to merge 1 commit into
masterfrom
testdirectory-stats

Conversation

@limakzi

@limakzi limakzi commented Sep 19, 2026

Copy link
Copy Markdown
Member

Adds two options to TestDirectory for getting at the per-file numbers it
already measures.

returnStats := true returns a record with the totals and one entry per
test file (name, shortName, failures, time, gcTime, mem) rather
than a bool. statsFile := "<path>" writes the same data as JSON, before
exitGAP quits — which is what makes it usable from a package test run,
where the exit code is otherwise the only thing that escapes the process.

Byte counts are exact, unlike the StringOfMemoryAmount values in the
progress table.

Both options default off and the existing return value is unchanged.


Use case

A package suite driven by gap-actions/run-pkg-tests must quit GAP itself, so today the only thing escaping the process is the exit code.
The progress table is printed, but line-wrapped and rounded to three significant figures (40.3GB), so parsing it back loses the byte count.

With statsFile the run drops a machine-readable file in the checkout:

TestDirectory(DirectoriesPackageLibrary("mypkg", "tst"),
              rec(exitGAP := true, statsFile := "tst-timings.json"));              

which a later workflow step can upload or render into the job summary:

- name: Summarise test timings
  if: always()
  run: |
    jq -r '.files | sort_by(-.time)[]
           | "\(.shortName): \(.time) ms, \(.failures) failures"' \
      tst-timings.json >> "$GITHUB_STEP_SUMMARY"

The same file can be posted back to the pull request.

This makes per-file runtime and allocation trackable across commits — worthwhile for suites where single files run for minutes and allocate tens of GB.

Assisted-by: Claude Code (Opus 5)

TestDirectory measured runtime, GC time and allocation per test file, then
stored them in local arrays that were never read. The only way out was the
progress table, where StringOfMemoryAmount rounds to three significant
figures.

`returnStats` returns them as a record instead of a bool; `statsFile`
writes the same data as JSON. The file is written before `exitGAP` quits,
so a package test run driven by gap-actions can collect it — there the exit
code is otherwise the only channel out.

Both default off, so existing callers are unaffected.

Assisted-by: Claude Code (Opus 5)
@limakzi limakzi added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library labels Sep 19, 2026
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.55172% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.00%. Comparing base (75493f0) to head (2fb593c).

Files with missing lines Patch % Lines
lib/test.gi 46.55% 31 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6581      +/-   ##
==========================================
- Coverage   79.01%   79.00%   -0.02%     
==========================================
  Files         683      683              
  Lines      295340   295270      -70     
  Branches     8668     8666       -2     
==========================================
- Hits       233369   233284      -85     
- Misses      60156    60167      +11     
- Partials     1815     1819       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread lib/test.gi
esc := function(str)
str := ReplacedString(str, "\\", "\\\\");
str := ReplacedString(str, "\"", "\\\"");
return Concatenation("\"", ReplacedString(str, "\n", "\\n"), "\"");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, my only thought is, do we want a not-complete JSON implementation here (this isn't enough escaping I don't think, technically?), would it be better to just return a dictionary, and then someone can use the JSON package if they want to write out JSON (or if we decide we really need json output in core, we could add that generically)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree. We can manage that with CSV files as well.
Something that is structured is sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants