feat: gcx as the assertion engine#334
Open
zeitlinger wants to merge 109 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the initial OATS v2 “gcx-driven” execution stack (polling, case schema, discovery, seeding, assertion vocabulary, runner orchestration, and reporting), alongside a new oats-v2 entrypoint and an end-to-end integration test using a fake gcx.
Changes:
- Add core v2 runtime packages (
engine,signalcmd,seed,wait,assert,runner,report,cache) with unit tests. - Add v2 configuration + case parsing/validation via
discovery(oats.toml) andv2case(case YAML), plus user-facingV2.md. - Add
cmd/v2binary with integration test wiring the full pipeline (fake gcx + httptest OTLP stub).
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| wait/wait.go | New polling primitives (Until/While) for v2. |
| wait/wait_test.go | Unit tests for polling semantics. |
| v2case/case.go | v2 case YAML schema, parsing, and validation. |
| v2case/case_test.go | Tests for YAML parsing and validation errors. |
| V2.md | User-facing overview of v2 architecture and config shapes. |
| signalcmd/signalcmd.go | Translate v2 assertions into gcx CLI args + render helper. |
| signalcmd/signalcmd_test.go | Tests for arg building and shell-quoting behavior. |
| seed/seed.go | Inline OTLP/HTTP JSON seeding implementation. |
| seed/seed_test.go | Tests for OTLP send behavior and defaults. |
| runner/runner.go | Orchestrate seed → poll/assert → emit report events + cache integration. |
| runner/runner_test.go | Runner unit tests (stub exec + text reporter assertions). |
| runner/cache_integration_test.go | Integration coverage for skip-when-unchanged cache behavior. |
| report/text.go | Human-readable text reporter + optional GHA annotations. |
| report/ndjson.go | NDJSON event stream reporter with verbosity-based filtering. |
| report/event.go | Event vocabulary + shared types and verbosity enum. |
| report/report_test.go | Tests for text/NDJSON output and annotation deduping. |
| engine/engine.go | gcx executor abstraction + os/exec implementation. |
| engine/engine_test.go | Executor behavior tests (stdout/stderr/exit/timeout/context). |
| discovery/discovery.go | oats.toml loader/validator and plan expansion + filtering. |
| discovery/discovery_test.go | Tests for config validation, glob expansion, and filtering. |
| cache/cache.go | File-backed TTL cache keyed by case+fixture+versions. |
| cache/cache_test.go | Tests for hashing, TTL eviction, corrupt entry behavior, clear/evict. |
| assert/assert.go | Assertion vocabulary (contains, not_contains, regex, value, count, absent). |
| assert/assert_test.go | Unit tests for assertion behavior and error cases. |
| cmd/v2/main.go | New oats-v2 CLI entrypoint wiring discovery → runner → report. |
| cmd/v2/integration_test.go | End-to-end integration test across v2 package boundaries. |
| cmd/v2/testdata/fake-gcx.sh | Fake gcx script for integration tests. |
| go.mod | Add TOML dependency for v2 discovery. |
| go.sum | Dependency checksums for TOML addition. |
| .github/renovate-tracked-deps.json | Track new Go module dependency for Renovate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- internal/cli: remove per-run oats-gcx-*.yaml temp config on fixture teardown; fail loudly instead of swallowing the write error - compose: start docker before the log-consumer goroutine so a failed Start can't leak a blocked reader - wait: capture timer by reference in cleanup defer; fix LastFailures doc - engine: correct Execute doc (--config/--context prepend order) - tests: relax wait overshoot bounds for CI, explicit SeedSettleDelay units, fix trimmed-vs-untrimmed engine assertions, rename cache test - docs: fix `oats -v` example (verbosity is -v=N) Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- cli: --fail-fast stops scheduling cases after the first failure - e2e: metrics+value pass/fail cases; fail-fast two-case suite - docs: README assertion reference, seed modes, custom-check contract; UPGRADING schema v2->v3 mappings; align example to attribute-list form Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- assert/profile + profile-fail: query LGTM's Pyroscope self-profile (runtime.mcall), no seeding needed — profiles can't be inline-seeded. Runs serially so the flamegraph is warm and CPU isn't contended. - assert/value: widen timeout to 60s (OTLP metric→Prometheus landing latency is 15-35s); validated against live otel-lgtm via the e2e harness Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- CLI moves to cobra: implicit-run root (bare `oats [flags]` still runs) plus run/list/migrate/cache clear/version subcommands. --list/--migrate kept as hidden deprecated flag aliases; verbosity is now -v/-vv/-vvv. - add `oats cache clear` (wipes the cache dir) - remove the `hermetic:` case field — it parsed but was never enforced - docs: README CLI section + UPGRADING (migrate subcommand, worked legacy→v3 example) Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- assert/{count,match-regexp,not-contains}-fail: FAIL-path coverage for the
assertion vocab (not-contains-fail settles first so the forbidden string is
present, otherwise not_contains trivially holds)
- format/ndjson: assert the NDJSON reporter emits run.end + pass count
- cli/migrate: drive `oats migrate` on a legacy yaml (no fixture needed)
- cli/cache-skip: run a suite twice against one cache dir; second run SKIPs
All validated against live otel-lgtm via the real e2e harness.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
fixture/compose-app-seed boots LGTM + a real instrumented app (telemetrygen, a prebuilt public image) that exports OTLP traces, then asserts the span is queryable through gcx. Covers the seed:app path that inline-otlp cases don't. Validated against a live compose stack. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
The cache clear subcommand had no end-to-end coverage; only the underlying Store.Clear() was unit-tested. Add a case that populates the cache with one green run, runs `oats cache clear`, then reruns and asserts the case re-executes (no SKIP line) — the sibling cache-skip case proves a rerun would skip without the clear. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Move suite fixture boot/teardown out of internal/cli into a new pluggable fixture package (fixture.go, compose.go, k3d.go). The CLI keeps suite orchestration and reporting; the package owns standing up compose/k3d/remote backends, waiting for readiness, and parallel-safety. Exports Runtime, Handle, Start, WaitForReady, SupportsParallel; compose/k3d/ gcx-config/token/port helpers stay unexported and are tested white-box. Pure refactor, no behavior change — unit, integration, and compose/k3d/remote e2e all pass unchanged. The previously dead waitForGrafanaToken seam (only kept alive by test overrides, never called in production) now has a real unit test. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- compose: start the docker command before spawning the stdout-reader goroutine (and close the pipe on Start error) so a failed Start can't leak a goroutine blocked on ReadString. - seed: make inline-otlp seeding context-aware (http.NewRequestWithContext, ctx threaded from Runner.RunCase) so a cancelled run stops seeding instead of delaying teardown. - yaml: propagate the filepath.Abs error in readTestCaseDefinition instead of panicking on the user-facing migrate path; document LoadTestCaseDefinition as a migrate-only shim, not supported public API. - go.mod: bump the go directive to 1.26.4 to match the toolchain mise pins. - wait: correct the Result.LastFailures docstring (it is returned on cancel/timeout when a prior asserter call reported failures). - casefile: rename TestValidate_RejectsPresentFalse to TestValidate_RejectsDuplicateAttributeKeys to match what it asserts. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
- fixture: give waitForHTTP an http.Client with a 10s timeout so a target that accepts TCP but never responds can't block a single probe past the overall readiness deadline. - compose: close the StdoutPipe read end when cmd.Start() fails in the logConsumer path, matching the background path and avoiding an FD leak. Addresses Copilot review comments. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
…mcall The assert/profile case matched runtime.mcall, a single scheduler frame only sampled when goroutines happen to be scheduled mid-sample. Under CI load it was frequently absent from Pyroscope's self-profile, flaking the case (repeated core shard failures). Match the broader runtime. prefix instead: any non-empty Go CPU profile contains many runtime.* frames, so the assertion still proves a real flamegraph came back through oats -> gcx -> Pyroscope without the single-frame fragility. Timeout unchanged. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
filepath.Abs returns an empty string on error, and the previous code overwrote filePath with it before formatting the error, yielding "failed to resolve path : ..." and losing the original input. Keep the original filePath for the message and only assign the resolved path on success. Addresses Copilot review comment. Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Member
|
Looks like the required statuses got broken? |
Member
Author
no - it's just got renamed to "build" |
Member
|
Well, "broken" - the PR is unmergeable without it or the repo settings being updated. |
Comment on lines
128
to
135
| @@ -100,12 +135,13 @@ func (c *Compose) runDocker(cc command) error { | |||
| } | |||
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
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.
WIP — tracking PR for the OATS v2 major rewrite.
Design + implementation plan: see internal-docs#14.
Direction
v2 replaces the bespoke TraceQL / PromQL / LogQL HTTP query infrastructure with
gcx.This branch no longer carries the old root YAML runner additively: the root
oatsbinary now routes through the gcx-driven CLI, and the acceptance coverage in this
PR is being migrated onto that path.
What's landed in this branch
engine/assert/contains/not_contains/regex/value/count/absentseed/wait/Until/Whilepolling (replaces gomega.Eventually)report/discovery/runner/cache/internal/cli/migrate/tests/e2e/All packages have tests; everything green; golangci-lint clean.
What's still to do in this PR
Compatibility
This rewrite drops the old public Go API (
yaml.ReadTestCases,yaml.RunTestCase) andthe legacy “pass one or more old yaml files directly to
oats” runner. Consumers nowmigrate via
oats --migrate ...plus the newoats.toml+oats-schema-version: 3case format.
See README.md for current user-facing docs.
Do not merge
This PR will stay in draft until the full v2 plan ships.