-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add test count summary to display total and skipped tests #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Created CountingTestLogger wrapper to track test counts - Wraps existing TestLogger implementations transparently - Counts total tests started and tests skipped - Prints summary at end: 'Test Summary: X total, Y skipped, Z ran' - Summary appears regardless of pass/fail status - Thread-safe using mutex for concurrent test execution Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Could this functionality be directly added to the existing logger instead of being a wrapper? |
|
Thank you for the feedback! I considered both approaches and went with the wrapper pattern for a few reasons: Wrapper approach (current implementation):
Direct integration alternative:
That said, I'm happy to refactor to the direct approach if that's the team's preference! The main tradeoff is between:
Which approach would you prefer for this codebase? |
🤖 I have created a release *beep* *boop* --- ## [2.33.0](v2.32.1...v2.33.0) (2025-10-31) ### Features * Add test count summary to display total and skipped tests ([#307](#307)) ([7662be6](7662be6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Summary Adds a test count summary that displays at the end of each test run showing the total number of tests attempted, how many were skipped, and how many actually ran. ## Changes - **New `CountingTestLogger` wrapper** (`framework/ldtest/test_logger.go`): - Wraps any existing `TestLogger` implementation - Tracks test counts by intercepting `TestStarted` and `TestSkipped` calls - Thread-safe using `sync.Mutex` for concurrent test execution - Provides `GetCounts()` method to retrieve counts - **Updated test execution** (`main.go`): - Wraps the configured test logger with `CountingTestLogger` - Prints summary after test completion: `Test Summary: X total, Y skipped, Z ran` - Summary appears regardless of pass/fail status ## Example Output ``` All tests passed Test Summary: 150 total, 12 skipped, 138 ran ``` ## Important Review Points 1. **No unit tests added**: While existing tests pass and the code compiles, I didn't add specific unit tests for the new `CountingTestLogger`. Consider whether these should be added. 2. **Output placement**: The summary appears after the existing pass/fail messages and any failure listings. Verify this placement meets expectations. 3. **Count interpretation**: - "Total" = all tests that were attempted (called `TestStarted`) - "Skipped" = tests filtered out or explicitly skipped (called `TestSkipped`) - "Ran" = Total - Skipped 4. **Cannot test with real SDK**: I verified the code compiles, passes existing tests, and passes linting, but couldn't test against an actual SDK test service to see the output in practice. --- **Requirements** - [ ] I have added test coverage for new or changed functionality *(No unit tests added for CountingTestLogger)* - [x] I have followed the repository's [pull request submission guidelines](../blob/master/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions *(Code compiles, existing tests pass, linting passes)* **Related issues** Requested by [email protected] in Slack thread: https://launchdarkly.slack.com/archives/C03DEV5CMFD/p1761927792275089 **Describe the solution you've provided** Implemented a transparent wrapper (`CountingTestLogger`) that tracks test counts by intercepting `TestStarted` and `TestSkipped` calls. The wrapper delegates all calls to the underlying logger, ensuring existing functionality remains unchanged. At the end of test execution, a summary line is printed showing total tests, skipped tests, and tests that actually ran. **Describe alternatives you've considered** 1. **Modifying the `Results` struct**: Could have added count fields to `Results`, but this would require more invasive changes throughout the test execution logic. 2. **Only showing total and skipped**: The requirement asked for "total ran" and "skipped", but showing all three numbers (total, skipped, ran) provides more clarity about what "ran" means. 3. **Different output format**: Could format as "X tests ran (Y skipped)" but the current format "X total, Y skipped, Z ran" is more explicit. **Additional context** Link to Devin run: https://app.devin.ai/sessions/61b07c9217f54e62848d32fc3b5136b1 Requested by: [email protected] --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]>
Summary
Adds a test count summary that displays at the end of each test run showing the total number of tests attempted, how many were skipped, and how many actually ran.
Changes
New
CountingTestLoggerwrapper (framework/ldtest/test_logger.go):TestLoggerimplementationTestStartedandTestSkippedcallssync.Mutexfor concurrent test executionGetCounts()method to retrieve countsUpdated test execution (
main.go):CountingTestLoggerTest Summary: X total, Y skipped, Z ranExample Output
Important Review Points
No unit tests added: While existing tests pass and the code compiles, I didn't add specific unit tests for the new
CountingTestLogger. Consider whether these should be added.Output placement: The summary appears after the existing pass/fail messages and any failure listings. Verify this placement meets expectations.
Count interpretation:
TestStarted)TestSkipped)Cannot test with real SDK: I verified the code compiles, passes existing tests, and passes linting, but couldn't test against an actual SDK test service to see the output in practice.
Requirements
Related issues
Requested by [email protected] in Slack thread: https://launchdarkly.slack.com/archives/C03DEV5CMFD/p1761927792275089
Describe the solution you've provided
Implemented a transparent wrapper (
CountingTestLogger) that tracks test counts by interceptingTestStartedandTestSkippedcalls. The wrapper delegates all calls to the underlying logger, ensuring existing functionality remains unchanged. At the end of test execution, a summary line is printed showing total tests, skipped tests, and tests that actually ran.Describe alternatives you've considered
Modifying the
Resultsstruct: Could have added count fields toResults, but this would require more invasive changes throughout the test execution logic.Only showing total and skipped: The requirement asked for "total ran" and "skipped", but showing all three numbers (total, skipped, ran) provides more clarity about what "ran" means.
Different output format: Could format as "X tests ran (Y skipped)" but the current format "X total, Y skipped, Z ran" is more explicit.
Additional context
Link to Devin run: https://app.devin.ai/sessions/61b07c9217f54e62848d32fc3b5136b1
Requested by: [email protected]