Feature/add skip support#18
Merged
Merged
Conversation
Introduce a first-class declarative skip system. Both `scenario` and `step` now accept any number of `skip_if` and `skip_unless` blocks. Each block may declare a `condition` expression and/or structured shortcuts (`os`, `arch`, `env_set`, `env`), plus an optional `reason`. This commit covers only the DSL surface: parsing, model types, and parse-time validation. Runtime evaluation, eval-context exposure of host info, reporters and CLI summaries land in follow-up commits. Notes on decoding contract: - Within a scenario or step, `skip_if` rules are emitted before `skip_unless` rules. Source order within a single kind is preserved. - A skip block with no actionable attribute (only `reason`) is rejected with a diagnostic to prevent silently-inert rules. - Unknown attributes inside a skip block are rejected with a diagnostic. Implementation note: `skipBlock` uses `hcl:",remain"` and walks attributes via `JustAttributes()` rather than tag-decoded `hcl.Expression` fields, because `gohcl` synthesises a non-nil expression for missing optional attributes, which would defeat presence detection.
Adds a new top-level `host` variable to every HCL expression evaluated
by the runtime, exposing the Go runtime OS and architecture:
host.os // e.g. "darwin", "linux"
host.arch // e.g. "amd64", "arm64"
This is the platform-detection primitive needed by the upcoming skip
rules so users can write:
skip_unless {
condition = host.os == "darwin"
reason = "iOS tests require macOS"
}
The object is intentionally minimal in V1 and will be extended later
(ci, hostname, ...) without breaking changes. Implemented in its own
helper to avoid name collision with Go's `runtime` package; callers
import as `goruntime` where needed.
Wires the parser-side skip_if / skip_unless blocks (commit 1) into the runtime. Both scenario-level and step-level rules are now honored: - Scenario-level: evaluated before DAG building. When a rule triggers, the scenario is marked StatusSkip with the resolved reason; no steps and no teardown run. - Step-level: evaluated in-goroutine before invoking the provider. When a rule triggers, the provider is not called and the step is recorded as StatusSkip with the resolved reason. Other steps in the layer keep running. Cascading skip to dependents lands in the next commit. Rule semantics: - Within a single block, populated attributes (os/arch/env_set/env/ condition) are combined with AND. - Across rules, the first triggering rule wins (skip_if rules are evaluated before skip_unless rules; source order within a kind is preserved). - Reasons fall back to a built-from-evaluation auto string when no `reason` attribute is provided. - Rule evaluation errors (bad types, non-bool condition, ...) become a StatusFail with kind="skip" rather than silently passing. Adds Scenario/Step.SkipReason to the report model so reporters can surface the explanation. Console/JSONL/JUnit/visual updates come in commit 5; today the existing teardown-skip path already shows up in output unchanged. Refactors runScenario by extracting per-step lifecycle into runOneStep + helpers to keep gocyclo under the 18 ceiling without inlining the layer execution.
When a step is cleanly skipped (skip_if / skip_unless triggered), any dependent step is now also marked SKIPPED with a reason of the form "depends on skipped step \"X\"" rather than being left to run with an empty captured result. The existing failure-cascade behavior (dependent of a failed step becomes skipped) is preserved and now also carries an explicit reason of the form "depends on failed step \"X\"" — previously the SkipReason was left empty. Introduces depTracker, a small concurrency-safe struct that records both failed and cleanly-skipped step names, exposing a single dependencyBlocker() lookup used by the per-step goroutine. This replaces the previous loose pair (failedSteps map + RWMutex) plumbed through several call signatures and naturally extends to the new skipped-step tracking.
Propagates the SkipReason carried by ScenarioResult and StepResult across every reporter so a skipped scenario or step is observable end-to-end, not only as a yellow status pill. Console: - Adds the missing scenarios-skipped column to the summary line: "scenarios N passed / N failed / N skipped, steps ...". Skipped scenarios were previously miscounted as failures. - Prints "reason: ..." under skipped scenarios and skipped steps when a SkipReason is set. JSONL: - Adds skip_reason to scenario events and step events when status is skipped. Backward-compatible: omitted when empty, so existing consumers see no schema change for the pass/fail majority. JUnit: - Adds a skipped="N" attribute on <testsuite>. - Adds a <skipped message="..."/> child on <testcase> for skipped scenarios. This matches Surefire / JUnit5 conventions so CI dashboards classify scenarios correctly. Visual: - Threads skip_reason through the JSON payload at scenario and step level (omitempty). The HTML/JS surfaces a step-level reason as a tooltip on the step header so the existing layout stays intact; styling-heavy treatment can land later.
Adds e2e/pass/skip_rules.tales exercising three flavours of the new skip system against the mockserver-backed e2e suite: - scenario-level skip_unless gated by an env value - step-level skip_unless gated by env_set, with cascade to a dependent step (proving the dependent reports a clear reason instead of failing on a missing capture) - a plain always-on scenario sitting next to the above, so the suite remains representative of normal traffic Verified locally: `make e2e` now ends with "scenarios 10 passed / 0 failed / 1 skipped, steps 25 passed / 0 failed, skipped 3" — exit code stays 0, and the skipped lines carry their reason in every reporter. Also adds skip_unless rules to every iOS scenario in e2e/ios (register, login, login_bad_account, failure_artifacts). The rule fires when os != darwin or IOS_APP_PATH is unset, which is exactly the precondition the iOS provider needs. The provider itself stays strict on non-darwin to keep accidental runs loud.
Adds docs/skip.md as the canonical reference for skip_if / skip_unless: scenario- and step-level usage, attribute matrix, host.os / host.arch / env semantics, the source-order / skip_if-before-skip_unless evaluation contract, the dependent cascade behavior, per-reporter rendering, and the unchanged exit code matrix. Also: - README.md: lists skip_if / skip_unless in the DSL highlights and adds the new "Top-level expression variables" subsection that documents host.os and host.arch alongside the existing config / result / request / response / input. - docs/mobile/ios.md: shows the recommended scenario-level skip_unless block in the DSL example so users on Linux / Windows do not need to depend on Makefile filtering. Explicitly notes that the provider stays strict — silent skips would mask coverage gaps. - .claude/skills/tales-test-generator/SKILL.md: lists the skip blocks alongside the other top-level constructs and exposes host.* in the top-level variables list so generated tests can use it. - CLAUDE.md: updates the DSL Surface section with the skip blocks, pointers to the runtime/skip.go and runtime/tracker.go implementation, and adds host.* to the available top-level variables.
There was a problem hiding this comment.
Pull request overview
Adds first-class skip_if / skip_unless support to the Tales DSL so scenarios and steps can be conditionally skipped based on host platform, env vars, or arbitrary expressions, while propagating skips to dependent steps and surfacing skip reasons across reporters.
Changes:
- Extend the parser/model schema to accept
skip_if/skip_unlessblocks on scenarios and steps, and add thehostexpression variable (host.os,host.arch). - Implement runtime skip-rule evaluation for scenarios/steps, plus dependency-based cascade skipping with distinct failed-vs-skipped tracking.
- Surface skip status/reason in console, JSONL, JUnit, and the visual report; add docs + e2e examples (notably gating iOS scenarios to macOS).
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Mentions skip rules and new host.* expression variables. |
| internal/runtime/tracker.go | Adds dependency tracker to distinguish failed vs skipped blockers. |
| internal/runtime/skip.go | Implements skip-rule evaluation and skip reason generation. |
| internal/runtime/skip_test.go | Unit tests for skip-rule matching and reason behavior. |
| internal/runtime/skip_integration_test.go | Runner-level integration tests for scenario/step skipping and cascades. |
| internal/runtime/runner.go | Wires scenario/step skipping into execution and dependency cascade behavior. |
| internal/report/visual/templates/visual.js | Shows step skip reason via tooltip in HTML report. |
| internal/report/visual/model.go | Adds skip_reason to visual JSON model for scenarios/steps. |
| internal/report/skip_test.go | Verifies skip reasons are emitted by console/JSONL/JUnit reporters. |
| internal/report/report.go | Adds SkipReason fields to scenario/step results. |
| internal/report/junit.go | Emits JUnit <skipped> elements and testsuite skipped="N". |
| internal/report/jsonl.go | Emits skip_reason for skipped scenario/step JSONL events. |
| internal/report/console.go | Adds skipped-scenario counting and prints skip reasons in console output. |
| internal/parser/skip.go | Decodes/validates skip blocks and attributes. |
| internal/parser/skip_test.go | Parser tests for skip blocks (scenario + step) and validation errors. |
| internal/parser/schema.go | Updates HCL schema to include skip blocks on scenarios/steps. |
| internal/parser/decode.go | Plumbs decoded skip rules into model scenario/step structs. |
| internal/model/step.go | Adds SkipRules to the step model. |
| internal/model/skip.go | Defines SkipKind/SkipRule model types. |
| internal/model/scenario.go | Adds SkipRules to the scenario model. |
| internal/lang/host.go | Implements host object (os, arch) for expressions. |
| internal/lang/host_test.go | Tests host.* availability in expression evaluation. |
| internal/lang/eval.go | Adds host to the EvalContext variables. |
| e2e/pass/skip_rules.tales | Adds e2e coverage/examples for skip rules and cascades. |
| e2e/ios/pass/register.tales | Gates iOS scenario on macOS + IOS_APP_PATH. |
| e2e/ios/pass/login.tales | Gates iOS scenario on macOS + IOS_APP_PATH. |
| e2e/ios/pass/login_bad_account.tales | Gates iOS scenario on macOS + IOS_APP_PATH. |
| e2e/ios/fail/failure_artifacts.tales | Gates iOS failure-artifacts scenario on macOS + IOS_APP_PATH. |
| docs/skip.md | Adds user documentation for the skip system. |
| docs/mobile/ios.md | Documents using skip rules to guard iOS scenarios in cross-platform CI. |
| CLAUDE.md | Updates DSL surface documentation to include skip rules + host var. |
| .claude/skills/tales-test-generator/SKILL.md | Updates skill guidance to include skip rules + host var. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses two Copilot review comments on PR #18: - evaluateSkipRules docstring claimed errors were "treated as not-triggering for safety", which contradicted the actual behavior: errors are returned to the caller and surface as a failed scenario/step with kind="skip". Rewrites the comment to describe what the code does and why (malformed rules must not be silently passed). - executeTeardownStep marked the teardown StatusSkip without a SkipReason when its `when` predicate evaluated false, leaving reporters with an empty reason field that the new StepResult contract claimed to populate. Sets SkipReason="when condition evaluated to false" so console / JSONL / JUnit / visual all surface a consistent explanation. Existing teardown tests assert only on Status and remain unchanged.
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.
No description provided.