Report duration, ordering, and noise cleanup; skip version check in result conversion#1996
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds report-template embedding to local builds, propagates version-check skipping, increases test-duration precision, centralizes report status filters, adds default result sorting, and exposes duration in report tables and TSV exports. ChangesBuild report embedding
Result conversion updates
Report result presentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Not up to standards ⛔
|
Deploying maester with
|
| Latest commit: |
3ab4615
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://22453802.maester.pages.dev |
| Branch Preview URL: | https://codex-improve-test-run-perfo.maester.pages.dev |
merill
marked this pull request as ready for review
July 25, 2026 09:13
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@powershell/public/Invoke-Maester.ps1`:
- Around line 319-335: Update the exclusion logic around $resolvedRunPaths so it
only targets canonical bundled Active Directory directories, not arbitrary
immediate “ad” children under caller-supplied roots. Handle a run path that
resolves directly to a bundled ad directory without constructing an ad/ad path,
and add coverage for both direct-directory and parent-root resolution cases.
In `@powershell/tests/functions/ActiveDirectoryOptIn.Tests.ps1`:
- Around line 98-102: Update the assertions in the unvalidated -Tag AD test
around Invoke-Maester to expect zero results, including result.TotalCount, while
preserving the marker and excluded-script assertions.
In `@report/src/lib/testSort.ts`:
- Around line 8-14: Update the severityOrder map so Critical ranks before High,
Medium, and Low, matching the table’s Critical, High, Medium, Low, Info ordering
while preserving the existing severity keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e7ad5d2a-eacf-4369-af18-7dfc1390505d
📒 Files selected for processing (12)
README.mdbuild/Build-LocalMaester.ps1build/Build-MaesterModule.mdpowershell/assets/ReportTemplate.htmlpowershell/internal/ConvertTo-MtMaesterResult.ps1powershell/public/Invoke-Maester.ps1powershell/tests/functions/ActiveDirectoryOptIn.Tests.ps1powershell/tests/functions/ConvertTo-MtMaesterResult.Tests.ps1report/src/components/TestResultsTable.jsxreport/src/lib/testSort.tsreport/src/pages/ExcelPage.tsxwebsite/docs/contributing.md
Resolves conflict in ConvertTo-MtMaesterResult.Tests.ps1 by keeping both the version-check skip test from this branch and the malformed-name warning tests from main (#1990). All 4 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rank Critical ahead of High in the report's default test ordering. It previously sorted below Low, which pushed the most severe findings down the default view. Scope the Active Directory path exclusion to directories where every test is AD-tagged, so unrelated tests in a folder named "ad" still run. Also handle a run path that is the "ad" directory itself, and back off to tag-only filtering when excluding the directory would leave Pester with no test files (Pester throws in that case). Format per-test Duration with milliseconds. Most tests finish in well under a second, so the new report column read 00:00:00 on nearly every row. Run-level totals keep the coarser format. Regenerate the embedded report template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Run.ExcludePath machinery saved ~237ms of Pester discovery on the real bundled test tree (513ms -> 276ms across 506 test files), not the 47% the synthetic 270-AD-test fixture suggested. That is well under 1% of a run against a live tenant, and it is all Pester-specific code that 3.0 removes. It also made AD the only tag that disappears from discovery. Every other filtered tag still reports as NotRun, and #1990 deliberately invested in rendering those rows well, so the exclusion cut against the current design. The real benefit was report noise: 270 of 699 tests were AD NotRun rows. Address that in the report instead by dropping NotRun from the default status selection. Rows stay in the data and remain selectable in the status filter, this hides filtered Preview and LongRunning tests too, and it matches the summary tile, which already subtracts NotRun from the total. Share the status lists through report/src/lib/testStatus.ts. The summary tiles treat the default selection as their unfiltered state, so clicking a tile on a freshly loaded report still narrows to that status. Keeps the rest of the PR: -SkipVersionCheck reaching ConvertTo-MtMaesterResult, the Duration column, the default severity and status ordering, and Build-LocalMaester.ps1 -BuildReport. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
merill
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@report/src/components/MtTestSummary.jsx`:
- Line 11: Update the status comparisons in MtTestSummary, including the
current-selection check and isActive calculation, to use set equality against
allStatuses rather than comparing only array lengths. Ensure selections with the
same size but different statuses are not treated as the default view or as an
active Total tests tile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6850a024-a153-48ee-939f-8e7128e53e7e
📒 Files selected for processing (6)
powershell/assets/ReportTemplate.htmlpowershell/public/Invoke-Maester.ps1report/src/components/MtTestSummary.jsxreport/src/components/TestResultsTable.jsxreport/src/lib/testStatus.tsreport/src/pages/HomePage.tsx
💤 Files with no reviewable changes (1)
- powershell/public/Invoke-Maester.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- report/src/components/TestResultsTable.jsx
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
1 participant
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.
Summary
-SkipVersionCheckswitch also preventConvertTo-MtMaesterResultfrom querying PowerShell Gallery, which could add up to 10 seconds while building report metadata.Durationto the report's Excel table and copied TSV output, formatted with milliseconds.NotRuntests by default in the report so tag-filtered tests do not crowd the view. They remain in the data and stay selectable in the status filter../build/Build-LocalMaester.ps1 -BuildReportto build the React report, copy the generated template into the PowerShell assets, then build, validate, and import the local module.ReportTemplate.htmland update local build documentation.Invoke-Maester.ps1changes by a single line in this PR (passing-SkipVersionCheckthrough).Report noise
A graph-only run discovers 699 tests in the bundled tree, of which 283 are reported as NotRun — 270 AD-tagged, 13 Preview/LongRunning. That is roughly 40% of the results table filled with tests the user did not ask to run.
NotRunis now excluded from the report's default status selection. This:The status lists moved into
report/src/lib/testStatus.tsso the table, the home page, and the summary tiles cannot drift. The tiles treat the default selection as their unfiltered state, so clicking a tile on a freshly loaded report still narrows to that status.Behavior changes
NotRundeselected. Selecting it in the status filter restores the previous view.Durationin the JSON output is nowhh:mm:ss.fffinstead ofhh:mm:ss. Most tests finish in well under a second, so the new column previously read00:00:00on nearly every row. Run-level totals keep the coarser format.Developer workflow
Validates that
npmis available, runs the production report build, verifiesreport/dist/index.html, copies it topowershell/assets/ReportTemplate.html, then performs the existing module build, validation, and import.Validation
powershell/tests/functions.tsc -bclean.Build-LocalMaester.ps1 -BuildReportcompleted, including the Vite production build, module validation, and local import.NotRunstill present in the status filter.