feat: always print summary after tests (like SimpleCov)#181
Merged
Conversation
Add `#summary` method to HTML reporter that prints screenshot count and report path after every test run: - "3 screenshots compared, 1 failure. Report: doc/screenshots/snap_diff_report.html" - "5 screenshots compared, no failures." - Nothing printed when no screenshots were taken Follows the SimpleCov pattern: always show status so developers know the reporter is active and where to find results. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 45 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
Contributor
Reviewer's GuideAdds a reusable summary API to the HTML reporter and updates the at_exit hook to always print a concise test run summary (screenshots count, failures, and report path) after tests, along with tests to cover the new behavior and finalize return values. Sequence diagram for at_exit HTML reporter summary printingsequenceDiagram
actor TestProcess
participant AtExitHook
participant ReportersRegistry as ReportersRegistry
participant HTMLReporter as HTMLReporter
participant Stdout as Stdout
TestProcess->>AtExitHook: process exits
AtExitHook->>ReportersRegistry: reporters = reporters.dup
loop each reporter
AtExitHook->>HTMLReporter: finalize()
HTMLReporter-->>AtExitHook: finalize_complete
AtExitHook->>HTMLReporter: summary()
alt summary present
HTMLReporter-->>AtExitHook: "[snap_diff] ..."
AtExitHook->>Stdout: puts summary
else no summary (total == 0)
HTMLReporter-->>AtExitHook: nil
end
end
Class diagram for HTML reporter with summary APIclassDiagram
class CapybaraScreenshotDiff {
}
class ReportersRegistry {
+Mutex reporters_mutex
+Array reporters
}
class HTMLReporter {
+Integer total
+Array failures
+Pathname output_path
+Float success_rate()
+String summary()
+String render()
+Pathname write_report()
+void finalize()
}
class AtExitHook {
+void run()
}
CapybaraScreenshotDiff --> ReportersRegistry : has
ReportersRegistry --> HTMLReporter : manages
AtExitHook --> ReportersRegistry : reads_reporters
AtExitHook ..> HTMLReporter : calls_finalize
AtExitHook ..> HTMLReporter : calls_summary
AtExitHook ..> AtExitHook : writes_to_stdout
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the
at_exithook you callreporter.summarytwice (puts reporter.summary if ... && reporter.summary); consider assigning it to a local variable to avoid duplicate work and ensure consistent output if the method ever gains side effects. - The summary string construction contains inline pluralization logic for both screenshots and failures; consider extracting a small helper (e.g.,
pluralize(count, singular, plural = nil)) to keep the method more readable and reduce duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `at_exit` hook you call `reporter.summary` twice (`puts reporter.summary if ... && reporter.summary`); consider assigning it to a local variable to avoid duplicate work and ensure consistent output if the method ever gains side effects.
- The summary string construction contains inline pluralization logic for both screenshots and failures; consider extracting a small helper (e.g., `pluralize(count, singular, plural = nil)`) to keep the method more readable and reduce duplication.
## Individual Comments
### Comment 1
<location path="test/unit/reporters/html_reporter_test.rb" line_range="165-174" />
<code_context>
+ assert_nil result
+ end
+
+ test "#summary returns screenshot count and status" do
+ reporter = HTML.new(output_path: @output_path)
+ reporter.record([build_passing_assertion("ok"), build_failing_assertion("fail")])
+ reporter.finalize
+
+ summary = reporter.summary
+ assert_includes summary, "1 failure"
+ assert_includes summary, "2 screenshots"
+ assert_includes summary, @output_path.to_s
+ end
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for pluralizing the failures label (e.g. "2 failures")
This test verifies the singular failure text and plural screenshots. To fully cover `summary`, please add another example with at least two failing assertions so the summary must include `"2 failures"`. That way both branches of the failures pluralization logic are tested and protected against regressions.
```suggestion
test "#summary returns screenshot count and status" do
reporter = HTML.new(output_path: @output_path)
reporter.record([build_passing_assertion("ok"), build_failing_assertion("fail")])
reporter.finalize
summary = reporter.summary
assert_includes summary, "1 failure"
assert_includes summary, "2 screenshots"
assert_includes summary, @output_path.to_s
end
test "#summary pluralizes failures label for multiple failures" do
reporter = HTML.new(output_path: @output_path)
reporter.record([
build_failing_assertion("first failure"),
build_failing_assertion("second failure")
])
reporter.finalize
summary = reporter.summary
assert_includes summary, "2 failures"
assert_includes summary, "2 screenshots"
assert_includes summary, @output_path.to_s
end
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
- Fix double summary evaluation: assign to local variable (was called twice) - Remove respond_to? guard: only HTML reporters exist, fail fast if not - Always warn on reporter errors (was silenced unless DEBUG) - Remove dead success_rate method (zero callers) - Fix fragile test assertion: check output_path instead of string "report" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
#summarymethod to HTML reporter that prints after every test runOutput examples
When no screenshots are taken (reporter not used or disabled), nothing prints.
Context
In jetthoughts.github.io, after upgrading to v1.12.0 with the HTML reporter enabled, there was no visible output confirming the reporter was working. SimpleCov always prints
Coverage report generated for ... to coverage/index.htmlregardless of pass/fail.Test plan
🤖 Generated with Claude Code
Summary by Sourcery
Add a summary output for the HTML reporter that always prints a concise test run status at exit.
New Features:
Tests: