Feat/action flag fix#45
Open
moshloop wants to merge 5 commits into
Open
Conversation
…--show-passed for non-test subcommands The flanksource/gavel composite action appends --no-progress --no-color --show-passed --format ... after the user-supplied $GAVEL_ARGS. Two issues prevented this from working: 1. cmd/gavel/test.go and cmd/gavel/test_framework_subcmds.go set testCmd.Flags().SetInterspersed(false), so any --flag token after a positional package path was treated as another path. With the action appending flags after user paths, every workflow that passed paths (e.g. `gavel test ./tests/e2e`) failed parsing with "starting path ./--no-progress does not exist". 2. action.yml unconditionally appended --show-passed, which is only defined on `gavel test` / `gavel lint`. Wrapping `gavel fixtures` (or any other subcommand) failed at flag parsing with "unknown flag: --show-passed". Fix both: - Flip SetInterspersed to true on `gavel test` and its framework subcommands. Cobra's `--` separator still terminates parsing for the documented `gavel test ./pkg -- --focus X` pass-through idiom. - Detect the first non-flag word in $GAVEL_ARGS in the action's run step and only append --show-passed when it is `test` or `lint`.
…-tag-aware discovery Lets callers run `gavel test ./... --ignore ./bench --ignore ./hack` instead of enumerating every package they want included. Bare directory patterns are recursive (no `/...` suffix needed); `/...` and `/**` suffixes are also accepted for Go-style compatibility. `.gitignore` is honored by default during test discovery (was already the case via WalkGitIgnoredBounded); pass `--no-gitignore` to opt out. Discovery now also evaluates build constraints via go/build.MatchFile so test files behind `//go:build never` (or any tag the current build context does not satisfy) no longer cause their package to be surfaced as runnable. The check is permissive on errors (missing files etc.) to preserve existing behavior on edge cases. Implementation notes: - `Ignore []string` and `NoGitignore bool` added to RunOptions; clicky's reflective flag binding picks them up from struct tags, no manual cobra wiring needed. - Filter is applied in TestOrchestrator.discoverPackagesInPaths after the per-runner discovery returns, so all frameworks benefit. - `--no-gitignore` is threaded through utils via a process-global toggle (utils.SetGitignoreDisabled) because the Runner interface does not pass per-call options into discovery; acceptable for a single-shot CLI.
Two follow-ups so duty's `./... --ignore ./hack` actually works: 1. Recognise Go-style recursive wildcards as starting paths. `./...`, `...`, `.`, and `./` now resolve to "discover from WorkDir" (the same code path used when no positional args are given), and any `./<dir>/...` is rewritten to `./<dir>` (DiscoverPackages already walks recursively). Previously gavel rejected `./...` with "starting path does not exist". 2. Apply --ignore to nested-module testGroups. expandNestedModuleGroups() spawns a separate group per child go.mod (e.g. ./hack/migrate, ./hack/generate-schemas), each running with its own WorkDir, so per-package filtering inside discoverPackagesInPaths never saw them. Drop ignored groups before runMultiRoot fans out. Also extends path_filtering_unit_test.go with TestExpandRecursiveWildcards and TestFilterIgnoredGroups.
…-passing sources Add discovery and merging of multiple gavel-* artifacts from different workflow runs or matrix shards. When a PR has multiple test jobs, each now appears as a separate entry in the results UI with per-job breakdowns, while aggregate counts roll up to the PR level. Also optimize the summary table to hide sources with 100% passing tests, collapsing them into a single "N more passing source(s)" row. This keeps PR comments short for the common "all green" case while still showing failing/skipped sources prominently. Key changes: - Add GavelArtifact struct and ListRunArtifacts/FindGavelArtifacts to discover artifacts via Actions API - Refactor DownloadArtifact to DownloadArtifactFiles for multi-file support - Add mergeGavelResults to concatenate test/lint/bench data from multiple payloads - Add GavelJobSummary for per-job breakdown in UI - Update summary table generation to filter and collapse all-passing sources - Add comprehensive tests for artifact discovery, merging, and table generation BREAKING CHANGE: DownloadArtifact now returns the first .json file only; use DownloadArtifactFiles for multi-file artifacts.
Gavel summary
Totals: 1779 passed · 14 failed · 17 skipped · 1m7s Failing testsgithub.com/flanksource/gavel/cmd/gavel — TestExecuteLintersJSCPDOptIngithub.com/flanksource/gavel/cmd/gavel — TestExecuteLintersJSCPDOptIn/jscpd_disabled_by_defaultgithub.com/flanksource/gavel/cmd/gavel — TestWriteSnapshotJSON_ReadByLoadResultsgithub.com/flanksource/gavel/cmd/gavel — TestLoadResults_MergesMultipleSnapshotsgithub.com/flanksource/gavel/cmd/gavel — TestRunUIServe_ReplaysSnapshotAndExits... and 7 more failing tests — see the full gavel-results artifact. Failing linterstsc — errortsc — 3 violation(s)
|
Implement a new index view that lists previously-saved test snapshots from the .gavel/ directory, allowing users to browse and replay historical test runs without re-running tests.
Key changes:
- Add /api/runs endpoint to list snapshots with metadata (test counts, lint violations, duration, git SHA)
- Add /api/runs/{name} endpoint to load individual snapshots, with pointer resolution for aliased runs (last, main, master)
- Add resolveGavelRoot() to discover .gavel/ directory when gavel ui serve is run with no arguments
- Implement RunIndex frontend component with sortable snapshot table showing test results and lint counts
- Add route parsing for /run/{name} URLs to support deep-linking into specific snapshots
- Maintain backward compatibility with existing /tests, /lint, etc. routes
Users can now run `gavel ui serve` in a git repo to browse all saved snapshots, or continue using `gavel ui serve <file>` to view a specific snapshot. Pointer files (last.json, main.json) are automatically resolved to their target snapshots.
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
1 participant
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.