DX-379 Enhance Flakeguard Console Output for Improved Developer Experience #1796
Conversation
…, improve rerun logic, and update logging for clarity. Adjusted command output messages for better user guidance and added checks for command-line argument usage to prevent unreliable test reruns.
lukaszcl
changed the title
|
Do the |
|
@jmank88 test-results.json contain only metadata for Flakeguard - Splunk integration. We have a separate JSON file with all logs for failed tests. But, this PR focuses on console experience (e.g. when you click a link to a build on Github CI). Here I'm planning to show logs like you see in the PR description. @jmank88 let me know what you think about it! |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the flakeguard run command to improve output clarity and execution flow while addressing configuration issues related to output directories.
- Introduces a centralized runState and outputManager for structured logging and improved run/rerun handling.
- Updates NewRunner calls in tests to include the new rawOutputDir parameter and refactors logging in main.go to avoid duplicate stderr output.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tools/flakeguard/runner/runner_test.go | Updated tests to pass the new rawOutputDir parameter as empty string. |
| tools/flakeguard/runner/runner_integration_test.go | Added temporary directory usage for raw output testing. |
| tools/flakeguard/runner/runner.go | Removed constant RawOutputDir and replaced it with instance-based configuration. |
| tools/flakeguard/main.go | Updated logger output to io.Discard to prevent duplicate logging. |
Comments suppressed due to low confidence (1)
tools/flakeguard/runner/runner_test.go:189
- [nitpick] Consider adding test cases that supply a non-empty rawOutputDir to verify that file path handling works correctly when an explicit directory is provided.
+ "",
|
I've taken a look, I don't see anything blatantly wrong which is all I'm good for when it comes to golang. Marking as ready for review. |
|
@jmank88 - does the above example output look good to you? |
4 participants
Summary
This PR significantly refactors the
flakeguard runcommand to improve output clarity, simplify execution flow, and fix configuration issues related to output directories. The goal is to provide a more user-friendly and robust testing experience.Changes
Output Overhaul:
outputManagerfor structured, step-by-step progress reporting (Prep, Initial Run, Retry, Summary).gotestsumlogs (initial and retry runs) into a single, final section for easier review.Short,Long) and documentation to reflect the new behavior and exit codes (0: success, 1: fail/flaky, 2: error).zerologoutput toio.Discardto prevent duplicate logging to stderr alongside the structured buffer output.Logic Simplification & Bug Fixes:
runStatestruct to manage configuration and results centrally.handleRerunsandhandleNoRerunsdirectly into the mainRunfunction for a cleaner flow.go test <file.go>edge case to skip unreliable reruns and provide clearer warnings.checkDependencies(go mod tidy) to log a warning and continue execution upon failure instead of exiting.getJSONOutputPathsreturns absolute file paths.Motivation
The previous output format of
flakeguard runwas less intuitive, mixing summary information with logs. The separation of run/rerun logic into helper functions also made the flow harder to follow. This refactor aims to:Rollout plan
After merging this PR we can do staged rollout.
Example logs
Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
The changes improve the FlakeGuard testing tool by enhancing its reporting capabilities, error handling, and overall usability. These modifications aim to provide clearer insights into test results, streamline the rerun process for failed tests, and offer a more structured output for easier analysis.
What
cmd/run.go:runStatestruct to hold run configuration and results.github.com/briandowns/spinner.outputManagerstruct for managing output buffer and exit codes.command-line-argumentstest packages to bypass rerun due to context loss.main.go:os.Stderrtoio.Discardto clean up console output.runner/runner.go:RawOutputDirtoRunnerstruct to specify directory for raw test output.NewRunnerfunction to acceptrawOutputDirparameter.runner/runner_integration_test.go:runner/runner_test.go: