DX-377 Refactor Flakeguard: Separate runner, executor, and parser concerns

[lukaszcl]

Motivation:

The original flakeguard/runner/runner.go had grown significantly, encompassing command execution, complex JSON parsing, error attribution, and result aggregation logic within a single file. This monolithic structure hindered maintainability, testability, and readability. This refactoring addresses these issues by separating concerns into dedicated components.

Changes:

1. Separation of Concerns: The core logic has been divided into distinct responsibilities within the flakeguard/runner tool directory:

   • runner package (runner.go): Acts as the primary orchestrator. It holds the main configuration, manages the overall workflow (initial runs, reruns), and delegates specific tasks to the executor and parser.
   • executor package (executor/executor.go): Defines an Executor interface and provides a default implementation (commandExecutor) responsible solely for executing go test and custom commands, isolating os/exec interactions and temporary output file handling.
   • parser package (parser/parser.go, parser/attribution.go):
     • Defines a Parser interface and provides a default implementation (defaultParser in parser.go).
     • Handles the complexities of parsing go test -json output streams, including managing transformations via the go-test-transform library.
     • Contains the intricate logic for detecting and attributing panics and races (moved to parser/attribution.go), along with associated regexes and error types.
     • Implements a robust multi-pass parsing strategy (Collect -> Process Per Test -> Aggregate/Finalize) to improve state management clarity and reliability compared to the previous single-pass approach.

2. Dependency Injection: Introduced Executor and Parser interfaces. The main Runner now accepts these interfaces via the NewRunner constructor, enabling easier unit testing by allowing mocks to be injected. Default implementations are used if nil is passed.

3. Configuration Handling: Introduced executor.Config and parser.Config structs to clearly delineate configuration options relevant to each component. The main Runner configures these based on its own settings. MaxPassRatio was removed from the Runner as it's considered a reporting/analysis concern.

4. Testing Enhancements:

   • Unit Tests: Added comprehensive unit tests for the critical parser package (parser_test.go, attribution_test.go), covering various JSON event scenarios, state transitions, panic/race attribution, subtest handling, run count correction, and output management.
   • Integration Tests: Adapted the existing integration tests (runner_integration_test.go) to use the refactored Runner. Added previously missing scenarios (panics, fail-fast) from the original runner_test.go to ensure end-to-end validation against the example_test_package.
   • Build Tags: Added a build tag (example_package_tests) to example_test_package to prevent its tests from running during standard flakeguard unit tests, cleaning up test execution logs. Integration tests now explicitly add this tag when invoking the Executor. This simplifies Flakeguard's Makefile.

---

Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.

Why

The changes made one at improving the handling of test execution and result parsing for a testing framework. These improvements include better structuring of test runner and parser logic, handling race conditions and panics in tests, and enhancing test result processing with additional attributes like timeout detection and improved error reporting.

What

• Runner now uses an Executor for running tests and a Parser for parsing test results, allowing for clearer separation of concerns and easier testing/mocking.
• Introduced executor and parser packages with interfaces and implementations for executing commands and parsing test results, respectively.
• Added handling for test panics, race conditions, and timeouts within the test execution and parsing logic, including attributes in the TestResult to indicate such issues.
• Introduced options for omitting outputs on successful tests and handling failures in parent tests differently, applicable during the parsing phase.
• Removed the previous testparser package and its logic, now integrated into the runner package with improved structure and clarity.
• Adjusted and streamlined test execution commands, including handling for running specific tests and rerunning failed tests with adjusted configurations.
• Enhanced parsing logic to handle edge cases and specific output patterns related to panics, race conditions, and timeouts, improving error attribution and reporting.

This restructuring and enhancement of the test execution and parsing logic aim to provide a more robust, clear, and flexible way of running tests and processing their results, especially in handling complex cases like panics and race conditions.