diff --git a/.claude/skills/act-test/SKILL.md b/.claude/skills/act-test/SKILL.md new file mode 100644 index 000000000..a2105da36 --- /dev/null +++ b/.claude/skills/act-test/SKILL.md @@ -0,0 +1,22 @@ +--- +name: act-test +description: Add or update focused automated coverage for plugin CI/CD workflow and action behavior +--- + +# Act testing + +Use this skill for behavioral changes to plugin-facing reusable CI/CD workflows, published plugin actions, and their supporting internal actions. Read the [act testing guide](../../../tests/act/CLAUDE.md) before changing tests. + +## Steps + +1. Identify the observable behavioral contract: outputs, artifacts, external request payloads, generated files, or workflow success/failure. +2. Select the narrowest credible coverage: static/unit coverage for pure wiring, a direct action test workflow for action-local behavior, a CI wrapper test, or a CD wrapper test. Existing coverage may already be adequate. +3. Isolate the smallest executable slice. Use `tests/act/internal/workflow/testing.go` mutations to retain only the relevant jobs, dependencies, and steps. Use the CI and CD wrapper helpers in `tests/act/internal/workflow/ci/ci.go` and `tests/act/internal/workflow/cd/cd.go` when testing the reusable wrappers. +4. Mock external boundaries, not the behavior under test. Use the existing GCS, Vault, GitHub App token, GCOM, Argo, HTTP-spy, artifact, and mockdata helpers in `tests/act/internal/act/` and `tests/act/internal/workflow/` as appropriate. +5. Assert outputs, artifacts, mocked files, summaries, or recorded HTTP requests before using `::act-debug::` annotations. Use annotations only when the observable contract cannot otherwise be asserted. +6. If act cannot faithfully emulate the behavior, document the precise limitation and the alternative validation in the PR description. Docker use alone is not a limitation. +7. Run the focused test first, then the broader relevant checks such as `make act-test`, `make act-lint`, or `make actionlint`. + +Do not add helper inputs merely to mirror workflow inputs; add them when a test needs to set them. + +Before local act execution, ensure the committed HEAD is pushed and remotely available; uncommitted working-tree changes are fine. Never push commits yourself. If the commit is unpushed, stop and ask the user to push it, explaining that act's `actions/checkout` clone requires the commit to exist remotely and will otherwise fail. Run `make clean` first when fixture-plugin `node_modules` directories exist. See the guide for details. diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md index 3baa7a1da..401becb26 100644 --- a/.claude/skills/code-review/SKILL.md +++ b/.claude/skills/code-review/SKILL.md @@ -31,6 +31,7 @@ changes, and consult other files in the repository as needed. - Ensure functions are focused, names are descriptive, and code is readable - Watch for duplication, dead code, or missing tests - Verify documentation and comments reflect the latest changes + - For changes to plugin-facing reusable CI/CD workflows, published plugin actions, or their supporting internal actions, confirm credible automated coverage, already-adequate coverage, or a specific act limitation and alternative validation documented in the PR description 4. **Review security and risk** - Look for injection points, insecure defaults, or missing validation - Confirm secrets or credentials are not exposed @@ -43,6 +44,7 @@ changes, and consult other files in the repository as needed. - [ ] Intended behavior works and matches requirements - [ ] Edge cases handled gracefully - [ ] Error handling is appropriate and informative +- [ ] Plugin-facing behavior changes have credible coverage, already-adequate coverage, or a specific limitation and alternative validation documented in the PR ### Code Quality @@ -75,6 +77,8 @@ These checks apply only to `.yml`/`.yaml` files in `.github/workflows/` and `act - Resource management, error handling, and logging reviewed - Suggested alternatives, additional test cases, or documentation updates captured +- Prefer `tests/act/` for orchestration and static or unit tests for pure wiring. This does not create a general coverage requirement for repository release or maintenance automation. +- Some Docker-based behavior is testable in this repository; do not treat Docker use alone as a reason coverage is impossible. Provide constructive feedback with concrete examples and actionable guidance for -the author. \ No newline at end of file +the author. diff --git a/.github/prompts/review-code.prompt.md b/.github/prompts/review-code.prompt.md index f4379612e..34686bc8c 100644 --- a/.github/prompts/review-code.prompt.md +++ b/.github/prompts/review-code.prompt.md @@ -22,7 +22,6 @@ You are reviewing code in the `grafana/plugin-ci-workflows` repository. This rep - `tests/act/` — Go testing framework using nektos/act in Docker to run workflows - Every other folder in `tests` (like `tests/simple-frontend`) — Dummy Grafana plugins used as test fixtures - `tests/act/mockdata/` — Pre-generated artifacts (built plugins, ZIPs) for fast test execution -- **The testing framework is WIP** — do not write new tests unless explicitly requested --- @@ -133,6 +132,12 @@ If files in `examples/base/**/*` are modified, remind that `examples/base/README If files in `tests/simple-*` (dummy test plugins) are modified, remind that `make mockdata` should be run to regenerate mock data. +### Plugin-Facing Behavior Coverage + +For behavioral changes to plugin-facing reusable CI/CD workflows, published plugin actions, or their supporting internal actions, verify that the PR has narrow, credible automated coverage where possible. Existing coverage may be adequate. Prefer `tests/act/` for orchestration and static or unit tests for pure wiring. + +If act cannot faithfully emulate the changed behavior, verify that the PR description names the specific limitation and the alternative validation. This policy does not create a general coverage requirement for repository release or maintenance automation. Some Docker-based behavior can be tested; do not reject coverage solely because Docker is involved. + --- ## Go Code (tests/act/) @@ -178,6 +183,6 @@ For every PR, verify: - [ ] New `actions/plugins/` actions added to `release-please-config.json` (both as a new package entry AND in the `.` package's `exclude-paths`) - [ ] `make genreadme` run if `examples/base/` modified - [ ] `make mockdata` run if `tests/simple-*` modified +- [ ] Plugin-facing behavior change has credible coverage, already-adequate coverage, or a specific act limitation and alternative validation documented in the PR - [ ] Go errors wrapped with context - [ ] `make act-lint` and `make actionlint` pass - diff --git a/AGENTS.md b/AGENTS.md index e066bb615..4cb0f9d52 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,97 +142,18 @@ When adding a new user-facing action in `actions/plugins/`, update `release-plea ## Testing Framework -The Go testing framework in `tests/act/` is **work in progress**. Do not write new tests unless explicitly requested. +Behavioral changes to plugin-facing reusable CI/CD workflows, published plugin actions, and the internal actions that support them should add or update the narrowest credible automated coverage whenever possible. This policy does not create a general testing requirement for repository release or maintenance automation. -Uses: testify for assertions, nektos/act in Docker, pre-built mockdata for speed. +Prefer `tests/act/` for workflow orchestration and static or unit tests for pure wiring. Existing coverage can suffice. If act cannot faithfully emulate the behavior, document the specific limitation and alternative validation in the PR description. See the [act testing guide](tests/act/CLAUDE.md) for selection, mocking, and limitation guidance. -When modifying test plugins in `tests/simple-*`, run `make mockdata` to regenerate mock data. - -### Go Testing Rules - -**Use testify, not stdlib testing:** -```go -import "github.com/stretchr/testify/require" - -func TestSomething(t *testing.T) { - result, err := DoSomething() - require.NoError(t, err) - require.Equal(t, expected, result) -} -``` - -**Use table-driven tests:** -```go -for _, tc := range []testCase{ - {name: "case1", input: "a", expected: "A"}, - {name: "case2", input: "b", expected: "B"}, -} { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - require.Equal(t, tc.expected, Transform(tc.input)) - }) -} -``` - -**Run independent tests in parallel** with `t.Parallel()`. - -**Workflow testing patterns:** See existing tests for current API usage: -- [main_smoke_test.go](tests/act/main_smoke_test.go) - Basic CI workflow tests -- [main_cd_test.go](tests/act/main_cd_test.go) - More complex CD workflow tests, with mocking and test workflow manipulation -- [main_backend_build_target_test.go](tests/act/main_backend_build_target_test.go) - Example of asserting on custom mage targets via `::act-debug::` annotations -- [internal/workflow/ci/ci.go](tests/act/internal/workflow/ci/ci.go) - CI workflow helpers -- [internal/act/act.go](tests/act/internal/act/act.go) - Runner implementation +The framework uses testify assertions, nektos/act in Docker, and pre-built mockdata for speed. It can exercise some Docker-based workflow behavior; do not treat Docker use itself as untestable. -**Asserting that a specific workflow step, configuration or edge case:** +For local act runs, ensure the commit under test is pushed and remotely available; uncommitted working-tree changes are acceptable. Run `make clean` before act tests when test-plugin `node_modules` directories exist. See the [act testing guide](tests/act/CLAUDE.md) for details. -Use the `::act-debug::` custom GHA command to emit a debug annotation from within a workflow shell step or a mage target. The test framework intercepts these and stores them as `AnnotationLevelDebug` annotations, which you can then assert on with `require.Contains`. - -Emit from a workflow step (logfmt format): -```yaml -- run: printf '::act-debug::msg="%s" key=%s\n' "my step ran" "${SOME_VAR}" - env: - SOME_VAR: ${{ inputs.something }} -``` - -Emit from a mage target (Go): -```go -fmt.Printf("::act-debug::msg=%q\n", "my target was invoked") -``` - -Assert in the test: -```go -expMsg, err := logfmt.MarshalKeyvals("msg", "my target was invoked") -require.NoError(t, err) -require.Contains(t, r.Annotations, act.Annotation{ - Level: act.AnnotationLevelDebug, - Message: string(expMsg), -}) -``` - -**Keeping act tests fast — skip irrelevant jobs and steps:** - -Act tests run real workflows in Docker, so they are slow by default. Always scope tests to the minimum necessary work using `MutateCIWorkflow().With(...)`: - -```go -ci.MutateCIWorkflow().With( - // Keep only the job under test; strip all other jobs and clear its `needs` - workflow.WithOnlyOneJob(t, "test-and-build", true), - // No-op steps that are irrelevant to what you're testing (e.g. frontend build) - workflow.WithNoOpStep(t, "test-and-build", "frontend"), - // Drop all steps after the one you care about (e.g. packaging, signing, GCS upload) - workflow.WithRemoveAllStepsAfter(t, "test-and-build", "backend"), -), -``` - -- `WithOnlyOneJob(t, jobID, removeDependencies)` — removes all jobs except `jobID`. Pass `true` to also clear its `needs` so it can run standalone. -- `WithNoOpStep(t, jobID, stepID)` — replaces a step with a shell no-op (`:`), leaving the step ID intact so subsequent steps that depend on its outputs don't break wiring. -- `WithRemoveAllStepsAfter(t, jobID, stepID)` — drops every step that comes after `stepID` in the job, cutting packaging, signing, upload, etc. +When modifying test plugins in `tests/simple-*`, run `make mockdata` to regenerate mock data. -When you only need to test backend behaviour, combine all three: skip the frontend step, stop after the backend step, and run only the `test-and-build` job. +Use `github.com/stretchr/testify/require`, prefer table-driven tests, and run independent tests in parallel with `t.Parallel()`. **Adding a new CI workflow input:** -1. Add the input to `WorkflowInputs` in [internal/workflow/ci/ci.go](tests/act/internal/workflow/ci/ci.go) and wire it in `SetCIInputs`. -2. Add the input to `.github/workflows/ci.yml` and pass it to the relevant action. -3. Add the input to `.github/workflows/cd.yml` and pass it through to `ci.yml`. -4. Add the input to the target composite action in `actions/internal/plugins/`. \ No newline at end of file +When a test needs to set a new CI input through the CI/CD wrapper helpers, add that input to `WorkflowInputs` in [internal/workflow/ci/ci.go](tests/act/internal/workflow/ci/ci.go) and wire it in `SetCIInputs`. Keep the workflow input and CD pass-through wiring correct where applicable; the helper does not need full input parity unless tests need it. diff --git a/README.md b/README.md index b2142b2a3..5083a2086 100644 --- a/README.md +++ b/README.md @@ -38,9 +38,10 @@ This repository is maintained by the Grafana Catalog team (`@grafana/grafana-cat ### Development Workflow 1. Make changes to workflows or actions -2. Test changes using the examples in your fork -3. Submit a pull request with your changes -4. Automated release via release-please upon merge +2. Run `make actionlint` for workflow changes; use `make act-lint` and `make act-test` for relevant act-test changes +3. Follow the [act testing guide](tests/act/CLAUDE.md) when workflow or action behavior needs automated coverage +4. Submit a pull request with your changes +5. Automated release via release-please upon merge ### Release Process diff --git a/tests/act/CLAUDE.md b/tests/act/CLAUDE.md new file mode 100644 index 000000000..40327c1d3 --- /dev/null +++ b/tests/act/CLAUDE.md @@ -0,0 +1,163 @@ +# Act testing guide + +`tests/act` exercises plugin CI/CD workflows with [nektos/act](https://github.com/nektos/act), Go, and Docker. Use it for behavior that depends on workflow orchestration. The framework builds temporary, narrowed workflow copies, runs them in act containers, and records workflow results for assertions. + +## Policy and selection + +For behavioral changes to plugin-facing reusable CI/CD workflows, published plugin actions, and the internal actions that support them, add or update the narrowest credible automated coverage whenever possible. Existing coverage can be sufficient. This policy does not impose a general coverage requirement on repository release or maintenance automation. + +Choose the smallest suitable level: + +1. **Static or unit coverage** for pure wiring: input declarations and pass-throughs, references, paths, and other behavior that does not require a running workflow. See `main_workflow_consistency_test.go` for examples. +2. **Direct action coverage** when a small test workflow can call the action and assert its contract without going through the CI/CD wrappers. +3. **CI wrapper coverage** for behavior in `.github/workflows/ci.yml`, using `internal/workflow/ci` helpers. +4. **CD wrapper coverage** when the behavior crosses `.github/workflows/cd.yml` and CI, using `internal/workflow/cd` helpers. + +Do not duplicate adequate existing coverage. If act cannot faithfully emulate the behavior, document the precise limitation and the alternative validation in the PR description. + +## Prerequisites and commands + +Run commands from the repository root. Workflow execution requires a working Docker daemon, the `act` CLI, Go, and either `GITHUB_TOKEN` or an authenticated `gh` CLI (`gh auth token`). `make act-lint` additionally requires `golangci-lint`. + +Act and `actions/checkout` clone the commit under test, so run act tests from a checkout whose committed HEAD is pushed and remotely available. A committed but unpushed change causes the clone to fail. Uncommitted working-tree changes are acceptable and do not cause this issue. + +```bash +make actionlint +make act-lint +make act-test + +# Focus on one test while iterating +cd tests/act && go test -v -timeout 1h -run '^TestGCS$' +``` + +`make act-test` runs `go test -v -timeout 1h` in `tests/act`. It may create temporary `act-*.yml` workflows and use temporary directories under `/tmp`. `make clean` removes act temporary data and cached action data. + +When a fixture plugin under `tests/simple-*` changes, regenerate its pre-built artifacts with `make mockdata`. + +## Framework capabilities + +The runner in `internal/act/act.go` maps this repository's workflow and action references to the local checkout, maps supported self-hosted runner labels to act's runner image, and exposes results through `RunResult`: + +- workflow success, step outputs, summaries, and GitHub Actions annotations; +- uploaded artifacts through `runner.ArtifactsStorage`; +- a local, read-only view of mocked GCS uploads through `runner.GCS.Fs`. + +The testing workflow model in `internal/workflow/testing.go` supports narrowed execution and mutations. It can keep one job, remove jobs, no-op or replace steps, stop after a step, provide dependency outputs, inject steps, override matrices, and alter step environments or workflow triggers. CI and CD wrapper builders are in `internal/workflow/ci/ci.go` and `internal/workflow/cd/cd.go`; they create nested temporary workflows so the called CI/CD workflows can be mutated. + +Event helpers in `internal/act/workflows.go` cover push, pull request (including fork and actor variants), pull-request-target, release, and workflow-dispatch payloads. Tests can use custom payloads when those helpers do not contain the required event fields. + +### Mocked boundaries + +Mock external boundaries, not the behavior under test. The checked-in helpers support: + +- GCS authentication/upload replacement with local file assertions; +- Vault-secret and GitHub App-token step replacements; +- GCOM HTTP handlers that receive requests from act containers; +- Argo-workflow trigger replacement backed by an HTTP request spy; +- generic Docker-accessible HTTP spies for request/input assertions; +- pre-built plugin `dist` and packaged artifact fixtures through `tests/act/mockdata` and CI wrapper helpers. + +Use the mock that preserves the changed contract. For example, mock a GCS service while asserting the paths emitted by the workflow, rather than no-oping the upload step when upload-path behavior is under test. + +## Go testing conventions + +Use `github.com/stretchr/testify/require` rather than standard-library assertions: + +```go +func TestSomething(t *testing.T) { + result, err := DoSomething() + require.NoError(t, err) + require.Equal(t, expected, result) +} +``` + +Prefer table-driven tests, and run independent subtests in parallel: + +```go +for _, tc := range []testCase{ + {name: "case1", input: "a", expected: "A"}, + {name: "case2", input: "b", expected: "B"}, +} { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.expected, Transform(tc.input)) + }) +} +``` + +## Writing focused tests + +Start from a representative test and keep the executable slice small: + +- `main_smoke_test.go` runs the basic CI workflow. +- `main_cd_test.go` exercises complex CD behavior with mocks and workflow mutations. +- `main_backend_build_target_test.go` demonstrates assertions through `::act-debug::` annotations. +- `main_context_test.go` narrows CI to the workflow-context step and varies event payloads. +- `main_gcs_test.go` uses mocked plugin artifacts and GCS, then asserts uploaded files and outputs. +- `main_cd_argo_test.go` asserts the mocked Argo request contract. +- `main_package_test.go` and `main_smoke_test.go` inspect uploaded artifacts. +- `main_workflow_consistency_test.go` performs static workflow and release-configuration consistency checks. +- `internal/workflow/ci/ci.go` provides CI workflow helpers. +- `internal/act/act.go` implements the runner. + +Prefer assertions on outputs, artifacts, mocked files, summaries, and recorded HTTP requests. The runner also captures annotations. + +### Debug annotations + +Use the custom `::act-debug::` annotation only when an observable contract cannot otherwise express the assertion. The framework stores it as an `AnnotationLevelDebug` annotation. + +Emit an annotation from a workflow step with logfmt: + +```yaml +- run: printf '::act-debug::msg="%s" key=%s\n' "my step ran" "${SOME_VAR}" + env: + SOME_VAR: ${{ inputs.something }} +``` + +Or emit one from a Mage target: + +```go +fmt.Printf("::act-debug::msg=%q\n", "my target was invoked") +``` + +Assert it in the test: + +```go +expMsg, err := logfmt.MarshalKeyvals("msg", "my target was invoked") +require.NoError(t, err) +require.Contains(t, r.Annotations, act.Annotation{ + Level: act.AnnotationLevelDebug, + Message: string(expMsg), +}) +``` + +### Keep act tests focused and fast + +Run only the job and steps that establish the behavior. Scope CI tests with `MutateCIWorkflow().With(...)`: + +```go +ci.MutateCIWorkflow().With( + // Keep only the job under test; strip all other jobs and clear its `needs`. + workflow.WithOnlyOneJob(t, "test-and-build", true), + // No-op steps that are irrelevant to the behavior under test. + workflow.WithNoOpStep(t, "test-and-build", "frontend"), + // Drop packaging, signing, upload, and later steps. + workflow.WithRemoveAllStepsAfter(t, "test-and-build", "backend"), +) +``` + +- `WithOnlyOneJob(t, jobID, removeDependencies)` removes all jobs except `jobID`. Pass `true` to clear its `needs` so it can run standalone. +- `WithNoOpStep(t, jobID, stepID)` replaces a step with the shell no-op `:` while retaining its ID and output wiring. +- `WithRemoveAllStepsAfter(t, jobID, stepID)` drops each following step in the job. + +Combine these mutations when only one behavior matters. Reuse mockdata rather than rebuilding fixture plugins, and keep independent Go subtests parallel when their resources are isolated. + +The repository is mounted into act's Docker container, so large local trees slow test startup. In particular, `node_modules` directories in fixture plugins under `tests/`, such as `tests/simple-frontend` and `tests/simple-backend`, add many files. Before running act tests when they exist, run `make clean`; its `clean-node-modules` target removes `node_modules` recursively beneath `tests/`. + +Add CI/CD wrapper helper inputs only when a test needs to set that input. The helpers do not need to mirror every workflow input merely for parity. + +## Limits and alternatives + +Act runs some Docker-based behavior successfully in this repository, including workflow steps that use the framework's mounted mockdata and Docker-accessible HTTP mocks. Docker use alone is not a limitation. + +However, act is not full GitHub Actions infrastructure. Concrete limitations can include job containers, Docker-in-Docker or Docker Compose, browser infrastructure, OIDC/workload identity federation, and fidelity to GitHub-hosted runners. Dynamic matrix behavior also has framework workarounds rather than faithful act execution. When one of these prevents credible coverage, state the exact unsupported behavior in the PR description and record the alternative validation performed, such as a targeted manual run in the appropriate environment or a static check of the wiring.