Repository navigation
feat(gotcha): advanced Go test runner with rich GitHub Actions integration - #1431
Closed
Erik Osterman (Cloud Posse) (osterman) wants to merge 229 commits into
Closed
Erik Osterman (Cloud Posse) (osterman) wants to merge 229 commits into
Erik Osterman (Cloud Posse) (osterman) wants to merge 229 commits into
Conversation
Add a Go-based test summary tool that parses `go test -json` output and generates markdown summaries for GitHub Actions. This makes it easier to identify test failures without searching through megabytes of test output. Features: - Parses Go test JSON output and preserves console output - Groups tests by status (failed, skipped, passed) for easy review - Displays test coverage with color-coded indicators (🟢≥80%, 🟡50-79%, 🔴<50%) - Shows every individual test (no rollup/grouping) - Provides copy-paste ready commands to reproduce failures locally - Works both in CI (GitHub Actions) and locally for development - Automatically writes to GITHUB_STEP_SUMMARY in CI or test-summary.md locally Changes: - Add tools/test-summary/main.go - standalone test summary generator - Update Makefile with new targets: testacc-summary, testacc-ci, testacc-view-summary, clean-test - Update GitHub Actions workflow to use make testacc-ci - Add test-results.json and test-summary.md to .gitignore 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
August 28, 2025 22:28
Contributor
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Erik Osterman (Cloud Posse) (osterman)
added
the
no-release
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1431 +/- ##
==========================================
- Coverage 55.95% 51.37% -4.59%
==========================================
Files 274 330 +56
Lines 28947 37077 +8130
==========================================
+ Hits 16198 19047 +2849
- Misses 10957 16071 +5114
- Partials 1792 1959 +167
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add constants to replace magic numbers and strings - Use nolint:forbidigo for os.Getenv (appropriate for standalone tool) - Split main() to avoid exitAfterDefer issue - Extract helper functions to reduce cognitive complexity - Pass TestEvent by pointer to avoid hugeParam warning - Fix octal literal format (0o644 instead of 0644) Refactored code into smaller, more focused functions: - setupUsage() - configure flag usage text - run() - main logic separated from main() - processLine() - handle single line of test output - recordTestResult() - record individual test result - categorizeResults() - sort results by status - sortResults() - sort test arrays - openOutput() - handle output file opening - openGitHubOutput() - GitHub-specific output handling - writeMarkdownContent() - write markdown output - writeCoverageSection() - write coverage with emoji - writeFailedTests(), writeSkippedTests(), writePassedTests() - write test sections 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Add multiple focused test files to improve maintainability and coverage: - testdata_test.go: Common test helpers and fixtures - parser_test.go: Tests for JSON parsing and test result processing - output_test.go: Tests for output generation and formatting - integration_test.go: End-to-end integration tests Key test coverage: - parseTestJSON() - handles various input formats - processLine() - JSON event processing - recordTestResult() - test result recording - categorizeResults() - test categorization by status - sortResults() - test result sorting - writeCoverageSection() - coverage display with emojis - shortPackage() - package name shortening - handleOutput() - output format routing - openInput() - input file handling Coverage: 35.0% of statements (good baseline) Also fixes remaining linting issues: - Reduced cyclomatic complexity by extracting helper functions - openInput(), handleOutput(), handleMarkdownOutput() 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
Bot
added
size/xl
Contributor
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
- Add GitHub Action steps to post test summaries as PR comments - Uses peter-evans/find-comment to locate existing comments - Uses peter-evans/create-or-update-comment to post/update summaries - Reads content directly from GITHUB_STEP_SUMMARY for consistency - Only runs on Linux matrix for pull request events 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add output_functions_test.go with tests for writeSummary, handleMarkdownOutput, setupUsage, openOutput, openGitHubOutput, writeMarkdownContent - Add test_sections_test.go with tests for writeFailedTests, writeSkippedTests, writePassedTests - Add main_functions_test.go with integration tests for main flow and run function - Add edge_cases_test.go with error handling and edge case tests - Remove unused containsNone function from testdata_test.go - Achieve 83.1% test coverage with all tests passing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change coverageMedThreshold from 50.0 to 40.0 - Update comments and test cases to reflect 40-79% yellow range - Red emoji now shows for <40%, yellow for 40-79%, green for 80%+ 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…vironment - Add environment variable isolation to TestOpenOutput/github_format - Add environment variable isolation to TestOpenGitHubOutput/local_mode - Add environment variable isolation to TestOpenGitHubOutput/custom_output_file - Tests now unset GITHUB_STEP_SUMMARY temporarily to ensure local mode behavior - Fixes failing tests in GitHub Actions CI where GITHUB_STEP_SUMMARY is set 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add function-level coverage analysis showing untested functions - Implement Go-based mock filtering to eliminate shell dependencies - Add dual output for GitHub Actions (GITHUB_STEP_SUMMARY + persistent file) - Fix markdown table formatting issues (extraneous parenthesis) - Rename output file to test-summary.md for consistency - Update GitHub workflow to use persistent file for PR comments 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace hardcoded /tmp/ paths with t.TempDir() in edge_cases_test.go for cross-platform compatibility - Update run() function calls in integration and main function tests to match new 5-parameter signature - Remove manual os.Remove() calls as t.TempDir() handles cleanup automatically 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
On Windows, calling os.Stderr.Sync() on pipes or console handles can hang indefinitely. This was causing TestStreamReporter_ShowsCoverageInPackageSummary to timeout after 10 minutes in CI. The fix skips Sync() calls on Windows while still flushing on Linux/Mac for CI output visibility. Affected files: - stream_reporter.go: 3 Sync() calls - processor.go: 1 Sync() call - display.go: 2 Sync() calls
The go.mod file gets updated to go 1.24.0 due to local Go 1.25.0 development version. Since CI is now pinned to Go 1.23, this version mismatch doesn't affect builds.
Instead of skipping Sync() on Windows, remove these calls entirely because: 1. stderr is typically unbuffered or line-buffered already 2. Sync() is meant for regular files, not console streams 3. Calling Sync() on pipes (like in tests) causes hangs on Windows This is the proper fix - we were using Sync() incorrectly. The hangs occurred because tests redirect stderr to pipes for output capture, and pipes don't support the flush operation that Sync() performs. Removed all 6 os.Stderr.Sync() calls and associated runtime checks.
…ling - Add output.Writer abstraction to manage stdout/stderr streams - Automatically unify output to stdout in GitHub Actions to prevent jumbling - Keep streams separate in terminal mode for proper piping - Update all stream components to use the new Writer abstraction - Remove all os.Stderr.Sync() calls (unnecessary and problematic) - Fix test files to use updated NewStreamReporter signature This ensures clean, non-interleaved output in CI environments while maintaining proper stdout/stderr separation for terminal usage.
- Count package build failures in the failed test counter - Update messages to clarify 'build error or initialization failure' - Fix nonsensical database skip reason in test example - Ensures exit code matches reported failures This fixes the issue where gotcha reported '0 failures' despite packages failing to build, which caused non-zero exit codes.
…in coverage display The vertical connector line was missing between file names and their functions in the coverage tree display, making it harder to visually connect which functions belong to which file. This was particularly noticeable for non-last files in the list. Added a vertical connector line that appears between the file name and its first function to improve the visual hierarchy.
- Remove duplicate test summary display in stream_orchestrator - Fix PrintUI calls to use format strings (fixes compilation errors) - Test summary is now only displayed once via stream processor - Ensures consistent failure counts including build failures This fixes the issue where two different test summaries were displayed with different failure counts, and resolves compilation errors from non-constant format strings.
The displayFinalTestSummary function was no longer being called after we fixed the duplicate summary issue. Removed the dead code to keep the codebase clean. Note: Some direct fmt.Printf calls remain in: - internal/coverage/processor.go (needs refactoring to use Writer) - internal/output/output.go (marked as TODO, used as fallback) These will be addressed in future refactoring.
- Add Writer field to StreamConfig for passing through execution flow - Update coverage processor functions to accept Writer parameter - Replace all fmt.Printf/Println calls with writer.PrintData - Update all call sites to pass Writer to coverage functions - Ensure exact output format is preserved (no visual changes) This completes the output abstraction, ensuring all output (including coverage reports) goes through the unified Writer interface. This prevents output interleaving in GitHub Actions and maintains proper stream separation in terminal environments. All tests pass and compilation succeeds.
- Move error logging before summary printing to prevent log messages from appearing in the middle of formatted test output - Update tests to use custom output.Writer for consistent behavior between local and GitHub Actions environments - Fix 8 failing tests that were capturing stderr instead of using the unified output stream - Maintain proper Error/Warn log levels for test failures and non-zero exit codes The error messages now appear in the correct sequence without interfering with the formatted test summary output.
✅ Test Results (atmos/windows)
⏭️ Skipped Tests (10)Click to see skipped tests
📊 Test Coverage
⏱️ Slowest Tests (20)
📦 Package Summary (43 packages)
Total Time: 131.90s |
The parser was not correctly extracting skip reasons from test output, causing GitHub summaries and comments to show "_No reason provided_" even when skip reasons were available. Fixed the parser to handle both patterns: 1. Skip reason appearing BEFORE --- SKIP line (common with t.Skipf) 2. Skip reason appearing AFTER --- SKIP line (common with subtests) Also handles skip reasons with multiple colons in the message correctly. Added comprehensive tests to verify both patterns work correctly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
Closing this PR to create a new one with a more focused scope. The work will continue in a new PR. |
Erik Osterman (Cloud Posse) (osterman)
deleted the
testacc-job-summary
branch
September 16, 2025 00:59
Contributor
|
|
1 similar comment
Contributor
|
|
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
2 participants
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.
what
why
key features
project/platformfor matrix builds (e.g.,atmos/linux,gotcha/windows)technical details
go testtoolingCompound Discriminator System
Comments now include both project context and platform information:
atmosfor main project,gotchafor tools/gotchaatmos/linuxGOTCHA_PROJECT_CONTEXT: Override auto-detected project contextGOTCHA_JOB_DISCRIMINATOR: Platform/OS identifier (e.g.,linux,windows)Example Comment Headers
✅ Test Results (atmos/linux)- Main project tests on Linux❌ Test Results (gotcha/windows)- Gotcha tool tests on Windows✅ Test Results (atmos/ubuntu-latest)- Main project on GitHub Actions Ubuntu runnerimplementation highlights
linux) or compound (project/platform) formatsexample output
testing
references
prd/tools/gotcha-test-runner.prd.mdSummary by CodeRabbit
New Features
Documentation
Chores