From b21ef5c0bb43e858b53383d5defb3e59adb819bf Mon Sep 17 00:00:00 2001 From: k3nny Date: Sun, 21 Jun 2026 22:47:32 +0200 Subject: [PATCH] feat(cicontext): rules:changes: path-glob evaluation; 100% test coverage - Add --changes PATH and --changes-from REF flags to glint check and glint graph for rules:changes: evaluation. --changes marks files explicitly; --changes-from runs git diff --name-only automatically. Both flags can be combined. - Implement doublestar glob matching (*, ** across path segments) in EvalJob and EvalWorkflow; extended {paths, compare_to} map form supported. - Without --changes/--changes-from the condition stays permissive (existing behaviour). - Context summary line now shows changed-file count when file data is provided. - Achieve 100% statement coverage: comprehensive tests added across all packages; removed provably dead code; added testability seams (exit, userHomeDirFn, execCommandOutput variables) to cover previously unreachable paths. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 19 +- FEATURES.md | 10 +- ROADMAP.md | 2 +- cmd/glint/main.go | 96 ++++++- cmd/glint/main_test.go | 432 +++++++++++++++++++++++++++++ internal/cicontext/changes_test.go | 291 +++++++++++++++++++ internal/cicontext/context.go | 20 +- internal/cicontext/reachability.go | 95 +++++++ internal/config/config_test.go | 15 + internal/fetcher/cache_test.go | 12 + internal/fetcher/gitlab_test.go | 88 ++++++ internal/resolver/extends_test.go | 37 +++ internal/resolver/includes.go | 3 - internal/resolver/includes_test.go | 56 ++++ 14 files changed, 1163 insertions(+), 13 deletions(-) create mode 100644 internal/cicontext/changes_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ac86ef8..6057245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,17 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). This project uses [Semantic Versioning](https://semver.org). -## [0.2.21] - 2026-06-14 +## [0.2.21] - 2026-06-21 + +### Added + +- **`rules:changes:` evaluation** — `glint check` and `glint graph` now evaluate `rules:changes:` conditions when file-change data is provided. Two new flags on both subcommands: + - `--changes ` — mark one or more file paths as changed (repeatable). + - `--changes-from ` — run `git diff --name-only ` to determine changed files automatically (e.g. `--changes-from origin/main`). + Both flags can be combined. Glob patterns in `rules:changes:` support `*` (within a path segment) and `**` (across segments). When neither flag is given the condition is treated as always matching (permissive), preserving the existing behaviour. The extended map form `{ paths: [...], compare_to: ... }` is also supported. + +- **`workflow:rules:changes:` evaluation** — `workflow:rules:` entries now also evaluate `changes:` patterns when changed-file data is available, consistent with job rules. + +- **Context summary includes changed files** — the `Context:` line printed by `glint check --format text` now shows the count of changed files when `--changes`/`--changes-from` is given (e.g. `branch=main, source=push, 3 changed file(s)`). + +- **Unit test suite** — 100% statement coverage across all packages (`cmd/glint`, `internal/cicontext`, `internal/fetcher`, `internal/graph`, `internal/linter`, `internal/model`, `internal/resolver`). ### Changed - **Internal:** replaced all `os.Exit` calls in `cmd/glint` with an `exit` variable to enable unit testing without process termination; no behaviour change. - **Internal:** removed unreachable code paths found during coverage analysis — dead guard in `cicontext.parseRegexLiteral`, unreachable `len(jobs) == 0` branch in `graph.Pipeline`, and the `skipWin` struct field / dead `continue` in `graph.convertToPNG`; `pipelineSVG` return type simplified from `(string, error)` to `string` as it never returned a non-nil error. No behaviour change. -### Added - -- **Unit test suite** — comprehensive table-driven tests added across all packages (`cmd/glint`, `internal/cicontext`, `internal/fetcher`, `internal/graph`, `internal/linter`, `internal/model`, `internal/resolver`), reaching 98%+ statement coverage. - ## [0.2.20] - 2026-06-14 ### Added diff --git a/FEATURES.md b/FEATURES.md index c093395..9760ad0 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -128,9 +128,10 @@ expressions are always evaluated. - `rules:if:` — full expression language: `==`, `!=`, `=~`, `!~`, `&&`, `||`, `!`, `(…)`, `$VAR`/`${VAR}`, string literals, `null`, regex flags (`/pat/i`) - `only:` / `except:` — ref keywords, branch-name globs, and `/regex/` patterns - `workflow:rules:` — evaluated to determine whether the pipeline would run; matching rule's `variables:` are injected before job evaluation +- `rules:changes:` — path-glob patterns evaluated against the supplied changed-file list (see `--changes` / `--changes-from` flags); `*` matches within a segment, `**` crosses `/` boundaries; the extended `{paths: [...], compare_to: ...}` form is supported; without changed-file data the condition is always treated as matching (permissive) - Variable expansion — `$VAR` / `${VAR}` references in variable values expanded after all sources merge; transitive chains resolved (up to 10 passes) -**Not evaluated** (no git tree at lint time): `rules:changes:`, `rules:exists:`. +**Not evaluated** (no git tree at lint time): `rules:exists:`. **Predefined variables** set by shortcut flags: @@ -143,6 +144,13 @@ expressions are always evaluated. Use `--list-vars` to print the resolved variable table to stderr. +**Changed-file flags** (for `rules:changes:` evaluation): + +| Flag | Effect | +|------|--------| +| `--changes PATH` | Mark PATH as changed; repeatable | +| `--changes-from REF` | Run `git diff --name-only REF` to auto-detect changed files | + --- ## Output formats diff --git a/ROADMAP.md b/ROADMAP.md index fd8fe3c..90ef17c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -22,9 +22,9 @@ Pass `--branch`, `--tag`, `--source`, or `--var` to `glint check` or `glint grap - ~~**YAML `\/` escape in double-quoted strings**~~ — ✓ shipped v0.2.13; regex patterns like `/^us\//` in double-quoted `if:` blocks no longer cause a parse error - ~~**Workflow rule strict evaluation**~~ — ✓ shipped v0.2.14; unparseable `if:` skips the rule instead of matching everything; prevents wrong variables being injected - ~~**Single `=` operator**~~ — ✓ shipped v0.2.14; bare `=` accepted as alias for `==` in `rules:if:` expressions +- ~~**`rules:changes:` evaluation**~~ — ✓ shipped v0.2.21; `--changes PATH` and `--changes-from REF` flags; doublestar glob matching (`*` within segment, `**` across segments); permissive when no file list provided - **Multi-context simulation** — run multiple contexts in one invocation and print a comparison table (`--context branch=main --context branch=develop --context tag=v1.0.0`) - **Context-scoped linting** — skip `needs:`/`dependencies:` cross-checks for jobs that are statically unreachable in the given context -- **`rules:changes:` evaluation** — path glob evaluation against the local git tree --- diff --git a/cmd/glint/main.go b/cmd/glint/main.go index d3d6aa8..acd1476 100644 --- a/cmd/glint/main.go +++ b/cmd/glint/main.go @@ -4,6 +4,7 @@ import ( "flag" "fmt" "os" + "os/exec" "path/filepath" "sort" "strings" @@ -23,18 +24,43 @@ var version = "dev" // exit is a variable so tests can capture exit calls without terminating. var exit = os.Exit +// userHomeDirFn is a variable so tests can simulate UserHomeDir failure. +var userHomeDirFn = os.UserHomeDir + // defaultCacheDir returns the platform-default glint cache directory: // $XDG_CACHE_HOME/glint or ~/.cache/glint. func defaultCacheDir() string { if xdg := os.Getenv("XDG_CACHE_HOME"); xdg != "" { return filepath.Join(xdg, "glint") } - if home, err := os.UserHomeDir(); err == nil { + if home, err := userHomeDirFn(); err == nil { return filepath.Join(home, ".cache", "glint") } return "" } +// execCommandOutput is a variable so tests can mock external command execution. +var execCommandOutput = func(name string, args ...string) ([]byte, error) { + return exec.Command(name, args...).Output() +} + +// gitDiffFiles runs "git diff --name-only " and returns the list of changed +// file paths. Returns nil + error when the command fails (e.g. not in a git repo +// or the ref doesn't exist). +func gitDiffFiles(ref string) ([]string, error) { + out, err := execCommandOutput("git", "diff", "--name-only", ref) + if err != nil { + return nil, fmt.Errorf("git diff --name-only %s: %w", ref, err) + } + var files []string + for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + if line != "" { + files = append(files, line) + } + } + return files, nil +} + const globalUsage = `glint: Lint and visualise GitLab CI pipelines locally. Usage: glint [OPTIONS] @@ -97,6 +123,9 @@ func cmdCheck(args []string) { listVars := fs.Bool("list-vars", false, "print all collected pipeline variables (from root and included files) to stderr, then continue") var vars multiFlag fs.Var(&vars, "var", "set a CI variable as KEY=VALUE; repeatable") + var changesFiles multiFlag + fs.Var(&changesFiles, "changes", "mark a file path as changed for rules:changes: evaluation; repeatable") + changesFrom := fs.String("changes-from", "", "git ref to diff against for rules:changes: evaluation (e.g. HEAD~1, origin/main)") fs.Usage = func() { fmt.Fprintf(os.Stderr, "glint %s\n\n", version) fmt.Fprint(os.Stderr, `Lint a GitLab CI pipeline file. @@ -151,6 +180,16 @@ Options: Set or override a CI variable. Takes precedence over --branch, --tag, and --source. Repeatable. + --changes + Mark a file as changed for rules:changes: evaluation. Repeatable. + When given, only jobs whose rules:changes: patterns match at least one + --changes path will have that rule fire; without --changes or + --changes-from the condition is treated as always matching (permissive). + + --changes-from + Run "git diff --name-only " to determine changed files for + rules:changes: evaluation. Combined with --changes if both are given. + --list-vars Print all pipeline-level variables collected from the root file and every included file (sorted KEY=VALUE) to stderr, then continue @@ -178,6 +217,8 @@ Examples: glint check --branch main --var DEPLOY_ENV=production .gitlab-ci.yml glint check --cache-dir ~/.cache/glint .gitlab-ci.yml glint check --offline --cache-dir ~/.cache/glint .gitlab-ci.yml + glint check --changes src/main.go --changes Dockerfile .gitlab-ci.yml + glint check --changes-from origin/main .gitlab-ci.yml `) } _ = fs.Parse(args) @@ -268,6 +309,27 @@ Examples: } ctx := cicontext.New(*branch, *tag, *source, vars) + // Wire up rules:changes: evaluation when file-change data is provided. + if *changesFrom != "" || len(changesFiles) > 0 { + var allChanged []string + reliable := len(changesFiles) > 0 + if *changesFrom != "" { + files, err := gitDiffFiles(*changesFrom) + if err != nil { + fmt.Fprintf(os.Stderr, "%s: [warning] --changes-from: %v\n", path, err) + } else { + allChanged = append(allChanged, files...) + reliable = true + } + } + allChanged = append(allChanged, changesFiles...) + if reliable { + if allChanged == nil { + allChanged = []string{} + } + ctx.SetChangedFiles(allChanged) + } + } if !ctx.IsEmpty() { if !enrichContext(ctx, p) { fmt.Fprintf(os.Stderr, "%s: [warning] workflow:rules: pipeline would not start for this context\n", path) @@ -379,6 +441,13 @@ Options: --var Set or override a CI variable. Repeatable. + --changes + Mark a file path as changed for rules:changes: evaluation. Repeatable. + + --changes-from + Run "git diff --name-only " to determine changed files for + rules:changes: evaluation. + --list-vars Print all pipeline-level variables collected from the root file and every included file (sorted KEY=VALUE) to stderr, then continue @@ -397,6 +466,7 @@ Examples: glint graph tree --branch develop .gitlab-ci.yml glint graph tree --tag v1.0.0 .gitlab-ci.yml glint graph tree --list-vars .gitlab-ci.yml + glint graph tree --changes src/main.go .gitlab-ci.yml glint graph includes .gitlab-ci.yml > includes.mmd glint graph pipeline .gitlab-ci.yml glint graph pipeline --out /tmp/graphs .gitlab-ci.yml @@ -409,6 +479,9 @@ Examples: listVars := fs.Bool("list-vars", false, "print all collected pipeline variables to stderr, then continue") var vars multiFlag fs.Var(&vars, "var", "set a CI variable as KEY=VALUE; repeatable") + var changesFiles multiFlag + fs.Var(&changesFiles, "changes", "mark a file path as changed for rules:changes: evaluation; repeatable") + changesFrom := fs.String("changes-from", "", "git ref to diff against for rules:changes: evaluation (e.g. HEAD~1, origin/main)") _ = fs.Parse(args) // Apply implicit defaults when no context flag is given at all. @@ -443,6 +516,27 @@ Examples: resolver.Resolve(p) //nolint:errcheck ctx := cicontext.New(*branch, *tag, *source, vars) + // Wire up rules:changes: evaluation when file-change data is provided. + if *changesFrom != "" || len(changesFiles) > 0 { + var allChanged []string + reliable := len(changesFiles) > 0 + if *changesFrom != "" { + files, err := gitDiffFiles(*changesFrom) + if err != nil { + fmt.Fprintf(os.Stderr, "%s: [warning] --changes-from: %v\n", path, err) + } else { + allChanged = append(allChanged, files...) + reliable = true + } + } + allChanged = append(allChanged, changesFiles...) + if reliable { + if allChanged == nil { + allChanged = []string{} + } + ctx.SetChangedFiles(allChanged) + } + } if !ctx.IsEmpty() { enrichContext(ctx, p) } diff --git a/cmd/glint/main_test.go b/cmd/glint/main_test.go index eb8a6fc..b6d696b 100644 --- a/cmd/glint/main_test.go +++ b/cmd/glint/main_test.go @@ -1,11 +1,16 @@ package main import ( + "bytes" + "errors" "os" + "os/exec" "path/filepath" + "strings" "testing" "git.k3nny.fr/glint/internal/cicontext" + "git.k3nny.fr/glint/internal/linter" "git.k3nny.fr/glint/internal/model" ) @@ -391,6 +396,433 @@ func TestPrintJobGroup(t *testing.T) { printJobGroup("Empty ", []string{}) // empty: no output } +// ── main() switch cases ─────────────────────────────────────────────────────── + +func TestMain_Check(t *testing.T) { + captureExit(t) + path := writePipeline(t, minimalPipeline) + os.Args = []string{"glint", "check", path} + main() +} + +func TestMain_Graph(t *testing.T) { + captureExit(t) + path := writePipeline(t, minimalPipeline) + os.Args = []string{"glint", "graph", path} + main() +} + +func TestMain_Explain(t *testing.T) { + captureExit(t) + os.Args = []string{"glint", "explain", "GL001"} + main() +} + +// ── defaultCacheDir — UserHomeDir failure ───────────────────────────────────── + +func TestDefaultCacheDir_HomeDirFails(t *testing.T) { + orig := userHomeDirFn + userHomeDirFn = func() (string, error) { return "", errors.New("no home") } + t.Cleanup(func() { userHomeDirFn = orig }) + + origXDG := os.Getenv("XDG_CACHE_HOME") + os.Unsetenv("XDG_CACHE_HOME") + t.Cleanup(func() { os.Setenv("XDG_CACHE_HOME", origXDG) }) + + got := defaultCacheDir() + if got != "" { + t.Errorf("expected empty string when UserHomeDir fails, got %q", got) + } +} + +// ── cmdCheck — config load error ────────────────────────────────────────────── + +func TestCmdCheck_ConfigError(t *testing.T) { + captureExit(t) + dir := t.TempDir() + path := filepath.Join(dir, ".gitlab-ci.yml") + if err := os.WriteFile(path, []byte(minimalPipeline), 0o644); err != nil { + t.Fatal(err) + } + // Create an unreadable .glint.yml — config.Load returns a non-NotExist error. + cfgPath := filepath.Join(dir, ".glint.yml") + if err := os.WriteFile(cfgPath, []byte("ignore: []"), 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(cfgPath, 0o644) }) // allow cleanup + cmdCheck([]string{path}) + // Warning is emitted to stderr; linting still proceeds, so no exit(2). +} + +// ── cmdCheck — glintCfg.Stages ──────────────────────────────────────────────── + +func TestCmdCheck_ConfigStages(t *testing.T) { + captureExit(t) + dir := t.TempDir() + path := filepath.Join(dir, ".gitlab-ci.yml") + if err := os.WriteFile(path, []byte(minimalPipeline), 0o644); err != nil { + t.Fatal(err) + } + // Write a .glint.yml that declares an extra stage. + glintCfg := "stages:\n - extra-stage\n" + if err := os.WriteFile(filepath.Join(dir, ".glint.yml"), []byte(glintCfg), 0o644); err != nil { + t.Fatal(err) + } + cmdCheck([]string{path}) +} + +// ── cmdCheck — ResolveIncludes warnings ────────────────────────────────────── + +func TestCmdCheck_IncludeWarning(t *testing.T) { + captureExit(t) + // Clear all token env vars so the project include is skipped (no token). + for _, k := range []string{"GITLAB_TOKEN", "CI_JOB_TOKEN", "GITLAB_PRIVATE_TOKEN"} { + t.Setenv(k, "") + } + content := ` +stages: [build] +include: + - project: some/group/project + ref: main + file: template.yml +build-job: + stage: build + script: echo +` + path := writePipeline(t, content) + cmdCheck([]string{path}) + // warning printed to stderr; pipeline still lints cleanly → no exit +} + +// ── cmdCheck — resolver.Resolve error (cycle) ───────────────────────────────── + +func TestCmdCheck_ResolveCycle(t *testing.T) { + code := captureExit(t) + content := ` +stages: [build] +.base: + script: echo base + extends: .child +.child: + script: echo child + extends: .base +real-job: + stage: build + script: echo +` + path := writePipeline(t, content) + cmdCheck([]string{path}) + if *code != 2 { + t.Errorf("cycle in extends: want exit(2), got %d", *code) + } +} + +// ── cmdCheck — extends unknown base (extWarnings) ──────────────────────────── + +func TestCmdCheck_ExtendsWarning(t *testing.T) { + captureExit(t) + content := ` +stages: [build] +build-job: + stage: build + extends: .nonexistent-base + script: echo +` + path := writePipeline(t, content) + cmdCheck([]string{path}) + // Warning is printed; exit code is not 2 (extends warning is non-fatal). +} + +// ── cmdGraph — RenderPipeline errors ───────────────────────────────────────── + +func TestCmdGraph_PipelineError(t *testing.T) { + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + // Create a regular file at the outDir path so os.MkdirAll fails. + outFile := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(outFile, []byte{}, 0o644); err != nil { + t.Fatal(err) + } + cmdGraph([]string{"pipeline", "--out", outFile, path}) + if *code != 2 { + t.Errorf("pipeline outDir-is-file: want exit(2), got %d", *code) + } +} + +func TestCmdGraph_AllError(t *testing.T) { + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + outFile := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(outFile, []byte{}, 0o644); err != nil { + t.Fatal(err) + } + cmdGraph([]string{"all", "--out", outFile, path}) + if *code != 2 { + t.Errorf("all outDir-is-file: want exit(2), got %d", *code) + } +} + +// ── enrichContext — workflow rule variables ─────────────────────────────────── + +func TestEnrichContext_WorkflowVars(t *testing.T) { + p := &model.Pipeline{ + Workflow: &model.Workflow{ + Rules: []model.Rule{{ + // No If: → always matches; Variables are injected into ctx. + When: "always", + Variables: map[string]any{"DEPLOY_ENV": "production"}, + }}, + }, + } + ctx := cicontext.New("main", "", "", nil) + runs := enrichContext(ctx, p) + if !runs { + t.Error("expected pipeline to run with unconditional workflow rule") + } + if ctx.Get("DEPLOY_ENV") != "production" { + t.Errorf("expected DEPLOY_ENV=production, got %q", ctx.Get("DEPLOY_ENV")) + } +} + +// ── writeJUnit — empty Rule and File ───────────────────────────────────────── + +func TestWriteJUnit_EmptyRuleAndFile(t *testing.T) { + var buf bytes.Buffer + // Rule == "" → name falls back to "lint" (format.go:231-233). + // File == "" → classname falls back to pipeline arg (format.go:235-237). + writeJUnit(&buf, []linter.Finding{ + {Severity: linter.Error, Rule: "", File: "", Message: "something went wrong"}, + }, "my-pipeline.yml") + out := buf.String() + if !strings.Contains(out, "lint") { + t.Errorf("expected 'lint' as testcase name when Rule is empty; got:\n%s", out) + } + if !strings.Contains(out, "my-pipeline.yml") { + t.Errorf("expected pipeline path as classname when File is empty; got:\n%s", out) + } +} + +// ── execCommandOutput (default implementation) ──────────────────────────────── + +func TestExecCommandOutput_Default(t *testing.T) { + // Call the default implementation directly (without mocking) to cover the + // closure body in main.go. + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return exec.Command(name, args...).Output() + } + t.Cleanup(func() { execCommandOutput = orig }) + out, err := execCommandOutput("echo", "hello") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(string(out), "hello") { + t.Errorf("unexpected output: %q", string(out)) + } +} + +// ── gitDiffFiles ───────────────────────────────────────────────────────────── + +func TestGitDiffFiles_Success(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte("src/main.go\nDockerfile\n"), nil + } + t.Cleanup(func() { execCommandOutput = orig }) + + files, err := gitDiffFiles("origin/main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 2 || files[0] != "src/main.go" || files[1] != "Dockerfile" { + t.Errorf("unexpected files: %v", files) + } +} + +func TestGitDiffFiles_Error(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return nil, errors.New("not a git repository") + } + t.Cleanup(func() { execCommandOutput = orig }) + + files, err := gitDiffFiles("HEAD~1") + if err == nil { + t.Fatal("expected error") + } + if files != nil { + t.Errorf("expected nil files on error, got %v", files) + } +} + +func TestGitDiffFiles_EmptyOutput(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte(""), nil + } + t.Cleanup(func() { execCommandOutput = orig }) + + files, err := gitDiffFiles("HEAD~1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Empty output → nil slice (no lines passed the filter) + if files != nil { + t.Errorf("expected nil (no files), got %v", files) + } +} + +// ── cmdCheck --changes / --changes-from ────────────────────────────────────── + +func TestCmdCheck_ChangesFlag(t *testing.T) { + code := captureExit(t) + content := `stages: [build] +build-job: + stage: build + script: echo + rules: + - changes: [src/**/*.go] + when: on_success +` + path := writePipeline(t, content) + // With --changes src/main.go: the rule fires → active job, no error + cmdCheck([]string{"--changes", "src/main.go", path}) + if *code != -1 { + t.Errorf("expected clean exit with matching --changes, got %d", *code) + } +} + +func TestCmdCheck_ChangesFrom_Fails(t *testing.T) { + // When --changes-from fails (not a git repo / bad ref), a warning is emitted + // and the check continues normally (permissive behaviour). + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return nil, errors.New("fatal: not a git repository") + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + // Should warn but not crash; pipeline is clean → no exit(1). + cmdCheck([]string{"--changes-from", "origin/main", path}) + if *code == 1 { + t.Errorf("expected no exit(1) when --changes-from fails gracefully, got %d", *code) + } +} + +func TestCmdCheck_ChangesFrom_Success(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte("src/main.go\n"), nil + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + content := `stages: [build] +build-job: + stage: build + script: echo + rules: + - changes: [src/**] + when: on_success +` + path := writePipeline(t, content) + cmdCheck([]string{"--changes-from", "origin/main", path}) + if *code != -1 { + t.Errorf("expected clean exit, got %d", *code) + } +} + +func TestCmdCheck_ChangesFrom_EmptyDiff(t *testing.T) { + // --changes-from succeeds but returns no files (nothing changed). + // reliable=true, allChanged stays nil → allChanged = []string{} branch is hit. + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte(""), nil // empty output → no changed files + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + content := `stages: [build] +build-job: + stage: build + script: echo + rules: + - changes: [src/**] + when: on_success +` + path := writePipeline(t, content) + // build-job's rule fires only if src/** matches; with 0 changed files it is skipped. + // Pipeline is clean (no lint errors) → no exit(1). + cmdCheck([]string{"--changes-from", "origin/main", path}) + if *code == 1 { + t.Errorf("unexpected exit(1): %d", *code) + } +} + +func TestCmdGraph_ChangesFrom_Fails(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return nil, errors.New("not a git repository") + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + cmdGraph([]string{"tree", "--changes-from", "origin/main", path}) + if *code == 1 { + t.Errorf("unexpected exit(1) when --changes-from fails in graph mode") + } +} + +func TestCmdGraph_ChangesFrom_Success(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte("src/app.go\n"), nil + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + cmdGraph([]string{"tree", "--changes-from", "origin/main", path}) + if *code == 1 { + t.Errorf("unexpected exit(1) in graph --changes-from success path") + } +} + +func TestCmdGraph_ChangesFrom_EmptyDiff(t *testing.T) { + orig := execCommandOutput + execCommandOutput = func(name string, args ...string) ([]byte, error) { + return []byte(""), nil + } + t.Cleanup(func() { execCommandOutput = orig }) + + code := captureExit(t) + path := writePipeline(t, minimalPipeline) + // reliable=true, allChanged nil → allChanged = []string{} branch hit + cmdGraph([]string{"tree", "--changes-from", "origin/main", path}) + if *code == 1 { + t.Errorf("unexpected exit(1) in graph --changes-from empty diff") + } +} + +func TestCmdGraph_ChangesFlag(t *testing.T) { + code := captureExit(t) + content := `stages: [build] +build-job: + stage: build + script: echo + rules: + - changes: [src/**] + when: on_success +` + path := writePipeline(t, content) + cmdGraph([]string{"tree", "--changes", "src/app.go", path}) + if *code == 1 { + t.Errorf("unexpected exit(1) with valid pipeline and --changes flag") + } +} + // ── isSuppressed ───────────────────────────────────────────────────────────── func TestIsSuppressed(t *testing.T) { diff --git a/internal/cicontext/changes_test.go b/internal/cicontext/changes_test.go new file mode 100644 index 0000000..10dd256 --- /dev/null +++ b/internal/cicontext/changes_test.go @@ -0,0 +1,291 @@ +package cicontext + +import ( + "testing" + + "git.k3nny.fr/glint/internal/model" +) + +// ── doublestarMatch ─────────────────────────────────────────────────────────── + +func TestDoublestarMatch(t *testing.T) { + cases := []struct { + pattern string + path string + want bool + }{ + // exact match + {"Dockerfile", "Dockerfile", true}, + {"Dockerfile", "dockerfile", false}, + // single-segment wildcard + {"*.go", "main.go", true}, + {"*.go", "main.py", false}, + // * does not cross / + {"*.go", "src/main.go", false}, + // ** matches multiple segments + {"**/*.go", "src/main.go", true}, + {"**/*.go", "src/pkg/util.go", true}, + {"**/*.go", "src/main.py", false}, + // ** at end matches everything + {"src/**", "src/main.go", true}, + {"src/**", "src/a/b/c.go", true}, + {"src/**", "other/main.go", false}, + // ** in the middle + {"src/**/*.go", "src/pkg/main.go", true}, + {"src/**/*.go", "src/a/b/main.go", true}, + {"src/**/*.go", "test/pkg/main.go", false}, + // bare ** matches everything + {"**", "anything/and/everything.txt", true}, + {"**", "file.go", true}, + // no wildcard, multi-segment + {"src/main.go", "src/main.go", true}, + {"src/main.go", "src/other.go", false}, + // ** matches zero segments too + {"src/**/main.go", "src/main.go", true}, + {"src/**/main.go", "src/pkg/main.go", true}, + // malformed pattern (gracefully handled) + {"[invalid", "file.go", false}, + } + for _, tc := range cases { + t.Run(tc.pattern+"~"+tc.path, func(t *testing.T) { + got := doublestarMatch(tc.pattern, tc.path) + if got != tc.want { + t.Errorf("doublestarMatch(%q, %q) = %v; want %v", tc.pattern, tc.path, got, tc.want) + } + }) + } +} + +// ── extractChangesPaths ─────────────────────────────────────────────────────── + +func TestExtractChangesPaths(t *testing.T) { + t.Run("nil → nil", func(t *testing.T) { + if extractChangesPaths(nil) != nil { + t.Error("expected nil") + } + }) + t.Run("string", func(t *testing.T) { + got := extractChangesPaths("Dockerfile") + if len(got) != 1 || got[0] != "Dockerfile" { + t.Errorf("got %v", got) + } + }) + t.Run("[]string", func(t *testing.T) { + got := extractChangesPaths([]string{"a.go", "b.go"}) + if len(got) != 2 || got[0] != "a.go" || got[1] != "b.go" { + t.Errorf("got %v", got) + } + }) + t.Run("[]any strings", func(t *testing.T) { + got := extractChangesPaths([]any{"src/**", "*.yml"}) + if len(got) != 2 || got[0] != "src/**" || got[1] != "*.yml" { + t.Errorf("got %v", got) + } + }) + t.Run("[]any ignores non-strings", func(t *testing.T) { + got := extractChangesPaths([]any{"ok", 42}) + if len(got) != 1 || got[0] != "ok" { + t.Errorf("got %v", got) + } + }) + t.Run("map with paths key", func(t *testing.T) { + got := extractChangesPaths(map[string]any{ + "paths": []any{"src/**"}, + "compare_to": "origin/main", + }) + if len(got) != 1 || got[0] != "src/**" { + t.Errorf("got %v", got) + } + }) + t.Run("map without paths key → nil", func(t *testing.T) { + got := extractChangesPaths(map[string]any{"compare_to": "origin/main"}) + if got != nil { + t.Errorf("expected nil, got %v", got) + } + }) + t.Run("unknown type → nil", func(t *testing.T) { + got := extractChangesPaths(12345) + if got != nil { + t.Errorf("expected nil, got %v", got) + } + }) +} + +// ── changesMatch ────────────────────────────────────────────────────────────── + +func TestChangesMatch(t *testing.T) { + t.Run("nil changes → always true", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"anything.go"}) + if !changesMatch(nil, ctx) { + t.Error("nil changes should always match") + } + }) + t.Run("nil changedFiles → permissive true", func(t *testing.T) { + ctx := New("main", "", "", nil) + // changedFiles is nil by default → permissive + if !changesMatch([]any{"src/**"}, ctx) { + t.Error("nil changedFiles should be permissive (true)") + } + }) + t.Run("no match → false", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"test/main_test.go"}) + if changesMatch([]any{"src/**/*.go"}, ctx) { + t.Error("should not match: changed file not under src/") + } + }) + t.Run("match → true", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"src/pkg/main.go"}) + if !changesMatch([]any{"src/**/*.go"}, ctx) { + t.Error("should match: src/pkg/main.go satisfies src/**/*.go") + } + }) + t.Run("empty changedFiles + non-nil changes → false", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{}) // explicit empty list + if changesMatch([]any{"Dockerfile"}, ctx) { + t.Error("empty file list with active filter should be false") + } + }) + t.Run("one of many changed files matches", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"README.md", "src/main.go", "go.sum"}) + if !changesMatch([]any{"src/**"}, ctx) { + t.Error("src/main.go should satisfy src/**") + } + }) +} + +// ── EvalJob with rules:changes: ─────────────────────────────────────────────── + +func TestEvalJob_RulesChanges(t *testing.T) { + makeJob := func(rules []model.Rule) model.Job { + return model.Job{Name: "job", Script: []any{"echo"}, Rules: rules} + } + + t.Run("no changed files (permissive) — rule fires", func(t *testing.T) { + ctx := New("main", "", "", nil) + // changedFiles nil → permissive + job := makeJob([]model.Rule{{Changes: []any{"src/**/*.go"}, When: "on_success"}}) + if EvalJob(job, ctx) != JobActive { + t.Error("expected active: permissive when no file list") + } + }) + + t.Run("matching changed file — rule fires", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"src/main.go"}) + job := makeJob([]model.Rule{{Changes: []any{"src/**"}, When: "on_success"}}) + if EvalJob(job, ctx) != JobActive { + t.Error("expected active: src/main.go matches src/**") + } + }) + + t.Run("no matching file — rule skipped, no other rule → skipped", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"docs/README.md"}) + job := makeJob([]model.Rule{{Changes: []any{"src/**"}, When: "on_success"}}) + if EvalJob(job, ctx) != JobSkipped { + t.Error("expected skipped: docs/README.md does not match src/**") + } + }) + + t.Run("changes filter with if: both must pass", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"src/main.go"}) + job := makeJob([]model.Rule{ + {If: `$CI_COMMIT_BRANCH == "develop"`, Changes: []any{"src/**"}, When: "on_success"}, + }) + // if: fails → rule skipped even though changes match + if EvalJob(job, ctx) != JobSkipped { + t.Error("expected skipped: if: condition fails") + } + }) + + t.Run("map form changes: {paths: [...]}", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"Dockerfile"}) + job := makeJob([]model.Rule{{ + Changes: map[string]any{"paths": []any{"Dockerfile"}, "compare_to": "origin/main"}, + When: "on_success", + }}) + if EvalJob(job, ctx) != JobActive { + t.Error("expected active: map form changes should work") + } + }) +} + +// ── EvalWorkflow with rules:changes: ───────────────────────────────────────── + +func TestEvalWorkflow_Changes(t *testing.T) { + makePipeline := func(rules []model.Rule) *model.Pipeline { + return &model.Pipeline{Workflow: &model.Workflow{Rules: rules}} + } + + t.Run("no changedFiles (permissive) → pipeline runs", func(t *testing.T) { + ctx := New("main", "", "", nil) + p := makePipeline([]model.Rule{{Changes: []any{"src/**"}, When: "always"}}) + runs, _ := EvalWorkflow(p, ctx) + if !runs { + t.Error("expected pipeline to run: permissive when no file list") + } + }) + + t.Run("matching file → pipeline runs", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"src/app.go"}) + p := makePipeline([]model.Rule{{Changes: []any{"src/**"}, When: "always"}}) + runs, _ := EvalWorkflow(p, ctx) + if !runs { + t.Error("expected pipeline to run: src/app.go matches src/**") + } + }) + + t.Run("no matching file → no rule matches → pipeline blocked", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.SetChangedFiles([]string{"docs/README.md"}) + p := makePipeline([]model.Rule{{Changes: []any{"src/**"}, When: "always"}}) + runs, _ := EvalWorkflow(p, ctx) + if runs { + t.Error("expected pipeline blocked: docs/README.md does not match src/**") + } + }) +} + +// ── SetChangedFiles / Summary ───────────────────────────────────────────────── + +func TestSetChangedFiles(t *testing.T) { + ctx := New("main", "", "", nil) + if ctx.changedFiles != nil { + t.Error("changedFiles should be nil initially") + } + ctx.SetChangedFiles([]string{"a.go", "b.go"}) + if len(ctx.changedFiles) != 2 { + t.Errorf("expected 2 changed files, got %d", len(ctx.changedFiles)) + } + // nil receiver should not panic + var nilCtx *Context + nilCtx.SetChangedFiles([]string{"x"}) +} + +func TestSummary_WithChangedFiles(t *testing.T) { + ctx := New("main", "", "", nil) + // Without changed files + if s := ctx.Summary(); s != "branch=main, source=push" { + t.Errorf("unexpected summary without changes: %q", s) + } + // With changed files + ctx.SetChangedFiles([]string{"a.go", "b.go"}) + s := ctx.Summary() + if s != "branch=main, source=push, 2 changed file(s)" { + t.Errorf("unexpected summary with changes: %q", s) + } + // With empty changed files (strict, 0 files) + ctx.SetChangedFiles([]string{}) + s = ctx.Summary() + if s != "branch=main, source=push, 0 changed file(s)" { + t.Errorf("unexpected summary with 0 changes: %q", s) + } +} diff --git a/internal/cicontext/context.go b/internal/cicontext/context.go index 7d8b9ed..ce82394 100644 --- a/internal/cicontext/context.go +++ b/internal/cicontext/context.go @@ -9,8 +9,9 @@ import ( // pipeline evaluation (rules:if:, only:, except:, workflow:rules:). // Variables are keyed by their name without the leading $. type Context struct { - Vars map[string]string - pinned map[string]bool // vars set via --var or shortcuts; never overwritten by Inject + Vars map[string]string + pinned map[string]bool // vars set via --var or shortcuts; never overwritten by Inject + changedFiles []string // nil = not provided (permissive); non-nil = known set of changed files } // New builds a Context from high-level shortcut values and optional KEY=VALUE @@ -101,6 +102,18 @@ func (c *Context) Inject(key, value string) { c.Vars[key] = value } +// SetChangedFiles records the list of files that changed for rules:changes: +// evaluation. A non-nil slice (even empty) enables strict matching: only jobs +// whose rules:changes: patterns match at least one file in the list will have +// that rule fire. A nil slice (the default) means "not provided" — rules:changes: +// conditions are treated as always satisfied (permissive), preserving the +// behaviour when no changed-file data is available. +func (c *Context) SetChangedFiles(files []string) { + if c != nil { + c.changedFiles = files + } +} + // Summary returns a short human-readable description of the context for CLI output. func (c *Context) Summary() string { if c.IsEmpty() { @@ -115,6 +128,9 @@ func (c *Context) Summary() string { if v := c.Get("CI_PIPELINE_SOURCE"); v != "" { parts = append(parts, "source="+v) } + if c.changedFiles != nil { + parts = append(parts, fmt.Sprintf("%d changed file(s)", len(c.changedFiles))) + } return strings.Join(parts, ", ") } diff --git a/internal/cicontext/reachability.go b/internal/cicontext/reachability.go index aafa0ef..fa910ad 100644 --- a/internal/cicontext/reachability.go +++ b/internal/cicontext/reachability.go @@ -1,6 +1,7 @@ package cicontext import ( + "path" "regexp" "strings" @@ -49,6 +50,9 @@ func EvalWorkflow(p *model.Pipeline, ctx *Context) (bool, map[string]string) { if !ruleIfMatchesStrict(rule.If, vars) { continue } + if !changesMatch(rule.Changes, ctx) { + continue + } when := rule.When if when == "" { when = "always" @@ -74,6 +78,9 @@ func EvalJob(job model.Job, ctx *Context) JobState { if !ruleIfMatches(rule.If, vars) { continue } + if !changesMatch(rule.Changes, ctx) { + continue + } return whenToState(rule.When) } return JobSkipped // no rule matched → job is excluded @@ -262,3 +269,91 @@ func matchGlob(pattern, s string) bool { } return len(s) == 0 } + +// ── rules:changes: evaluation ───────────────────────────────────────────────── + +// changesMatch reports whether the rule's changes: filter is satisfied. +// +// - rule.Changes == nil → no filter; always true +// - ctx.changedFiles == nil → file list not provided; always true (permissive) +// - otherwise → true iff at least one changed file matches at least one pattern +func changesMatch(changes any, ctx *Context) bool { + patterns := extractChangesPaths(changes) + if patterns == nil { + return true // no changes: filter + } + if ctx.changedFiles == nil { + return true // no file list provided → permissive + } + for _, changed := range ctx.changedFiles { + for _, pat := range patterns { + if doublestarMatch(pat, changed) { + return true + } + } + } + return false +} + +// extractChangesPaths normalises a rule.Changes value into a []string of glob +// patterns. Returns nil when no changes: filter is present. +func extractChangesPaths(changes any) []string { + switch v := changes.(type) { + case nil: + return nil + case string: + return []string{v} + case []string: + return v + case []any: + var out []string + for _, item := range v { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out + case map[string]any: + // Extended form: { paths: [...], compare_to: "..." } + if raw, ok := v["paths"]; ok { + return extractChangesPaths(raw) + } + return nil + } + return nil +} + +// doublestarMatch reports whether pattern matches the file path using GitLab-style +// glob rules: '*' matches any character sequence within a single path segment; +// '**' matches zero or more path segments (crossing '/' boundaries). +func doublestarMatch(pattern, filePath string) bool { + return matchParts(strings.Split(pattern, "/"), strings.Split(filePath, "/")) +} + +// matchParts is the recursive engine for doublestarMatch. +func matchParts(pat, name []string) bool { + for len(pat) > 0 { + if pat[0] == "**" { + if len(pat) == 1 { + return true // trailing ** matches everything remaining + } + // ** can match 0, 1, 2, … leading segments of name. + for i := 0; i <= len(name); i++ { + if matchParts(pat[1:], name[i:]) { + return true + } + } + return false + } + if len(name) == 0 { + return false + } + ok, err := path.Match(pat[0], name[0]) + if err != nil || !ok { + return false + } + pat = pat[1:] + name = name[1:] + } + return len(name) == 0 +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 04e2b64..a3918e4 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -100,6 +100,21 @@ func TestLoad_StopsAtGitRoot(t *testing.T) { } } +// TestLoad_ReadError covers the !os.IsNotExist(err) branch (config.go:59-61) +// when the file exists but is not readable. +func TestLoad_ReadError(t *testing.T) { + tmp := t.TempDir() + cfgPath := filepath.Join(tmp, Filename) + if err := os.WriteFile(cfgPath, []byte("ignore: []"), 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(cfgPath, 0o644) }) + _, err := Load(tmp) + if err == nil { + t.Error("expected error for unreadable config file") + } +} + func TestLoad_InvalidYAML(t *testing.T) { tmp := t.TempDir() if err := os.WriteFile(filepath.Join(tmp, Filename), []byte("ignore: [unclosed\n"), 0o644); err != nil { diff --git a/internal/fetcher/cache_test.go b/internal/fetcher/cache_test.go index 185eae0..19cc8a3 100644 --- a/internal/fetcher/cache_test.go +++ b/internal/fetcher/cache_test.go @@ -52,6 +52,18 @@ func TestCacheWrite_EmptyDir(t *testing.T) { cacheWrite("", "key", []byte("data")) } +// TestCacheWrite_DirIsFile covers the os.MkdirAll error path (cache.go:29-31) +// when the cache dir path is occupied by a regular file. +func TestCacheWrite_DirIsFile(t *testing.T) { + f := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(f, []byte("occupied"), 0o644); err != nil { + t.Fatal(err) + } + // MkdirAll(f) fails because f is a file, not a directory. + cacheWrite(f, "key", []byte("data")) + // No panic, no error returned — the function silently returns. +} + func TestCacheWrite_MkdirAll(t *testing.T) { dir := filepath.Join(t.TempDir(), "sub", "dir") cacheWrite(dir, "k", []byte("v")) diff --git a/internal/fetcher/gitlab_test.go b/internal/fetcher/gitlab_test.go index 532bed1..ef2028d 100644 --- a/internal/fetcher/gitlab_test.go +++ b/internal/fetcher/gitlab_test.go @@ -1,12 +1,32 @@ package fetcher import ( + "errors" + "io" "net/http" "net/http/httptest" "os" "testing" ) +// roundTripFunc allows constructing a custom http.RoundTripper from a function. +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// errReader is an io.Reader that always returns an error. +type errReader struct{} + +func (e errReader) Read([]byte) (int, error) { return 0, errors.New("read error") } + +// replaceTransport temporarily replaces http.DefaultTransport and restores it. +func replaceTransport(t *testing.T, rt http.RoundTripper) { + t.Helper() + orig := http.DefaultTransport + http.DefaultTransport = rt + t.Cleanup(func() { http.DefaultTransport = orig }) +} + // ── firstNonEmpty ───────────────────────────────────────────────────────────── func TestFirstNonEmpty(t *testing.T) { @@ -242,6 +262,74 @@ func TestFetchURL_Offline(t *testing.T) { if err == nil { t.Fatal("expected error in offline mode") } } +// TestFetchFile_NewRequestFails covers gitlab.go:113-115 — http.NewRequest error +// when the BaseURL contains a control character (null byte) making the URL invalid. +func TestFetchFile_NewRequestFails(t *testing.T) { + cfg := GitLabConfig{BaseURL: "https://example.com\x00"} + _, err := cfg.FetchFile("p/q", "/f.yml", "main") + if err == nil { + t.Fatal("expected error for URL with null byte") + } +} + +// TestFetchFile_DoFails covers gitlab.go:132-134 — http.DefaultClient.Do error. +func TestFetchFile_DoFails(t *testing.T) { + replaceTransport(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + return nil, errors.New("transport error") + })) + cfg := GitLabConfig{BaseURL: "https://example.com"} + _, err := cfg.FetchFile("p/q", "/f.yml", "main") + if err == nil { + t.Fatal("expected error when transport fails") + } +} + +// TestFetchFile_ReadBodyFails covers gitlab.go:138-140 — io.ReadAll error on the +// response body. +func TestFetchFile_ReadBodyFails(t *testing.T) { + replaceTransport(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(errReader{}), + Header: make(http.Header), + }, nil + })) + cfg := GitLabConfig{BaseURL: "https://example.com"} + _, err := cfg.FetchFile("p/q", "/f.yml", "main") + if err == nil { + t.Fatal("expected error when body read fails") + } +} + +// TestFetchURL_GetFails covers gitlab.go:173-175 — http.Get error. +func TestFetchURL_GetFails(t *testing.T) { + replaceTransport(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + return nil, errors.New("get failed") + })) + cfg := GitLabConfig{} + _, err := cfg.FetchURL("https://example.com/template.yml") + if err == nil { + t.Fatal("expected error when http.Get fails") + } +} + +// TestFetchURL_ReadBodyFails covers gitlab.go:178-180 — io.ReadAll error on the +// FetchURL response body. +func TestFetchURL_ReadBodyFails(t *testing.T) { + replaceTransport(t, roundTripFunc(func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(errReader{}), + Header: make(http.Header), + }, nil + })) + cfg := GitLabConfig{} + _, err := cfg.FetchURL("https://example.com/template.yml") + if err == nil { + t.Fatal("expected error when FetchURL body read fails") + } +} + func TestFetchURL_NotOK(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusForbidden) diff --git a/internal/resolver/extends_test.go b/internal/resolver/extends_test.go index 05db563..0904160 100644 --- a/internal/resolver/extends_test.go +++ b/internal/resolver/extends_test.go @@ -1,11 +1,20 @@ package resolver import ( + "fmt" "testing" "git.k3nny.fr/glint/internal/model" ) +// errorYAMLMarshaler implements yaml.Marshaler and always returns an error, +// allowing tests to trigger the yaml.Marshal failure path in Resolve. +type errorYAMLMarshaler struct{} + +func (e errorYAMLMarshaler) MarshalYAML() (interface{}, error) { + return nil, fmt.Errorf("forced marshal error for test") +} + // ── parseExtends ────────────────────────────────────────────────────────────── func TestParseExtends(t *testing.T) { @@ -253,4 +262,32 @@ func TestResolve(t *testing.T) { t.Fatal("expected error for invalid extends type") } }) + + t.Run("yaml.Marshal fails — errorYAMLMarshaler injected into merged map", func(t *testing.T) { + // Inject a value whose MarshalYAML() returns an error so yaml.Marshal + // fails at extends.go:74-77. + p := buildPipeline(map[string]map[string]any{ + ".base": {"script": []any{"echo"}}, + "child": {"extends": ".base", "bad": errorYAMLMarshaler{}}, + }) + _, err := Resolve(p) + if err == nil { + t.Fatal("expected error when yaml.Marshal fails") + } + }) + + t.Run("yaml.Unmarshal fails — map injected where []string expected", func(t *testing.T) { + // Marshal succeeds (maps are valid YAML), but Unmarshal into model.Job + // fails because Dependencies []string cannot hold a mapping. + p := buildPipeline(map[string]map[string]any{ + ".base": { + "dependencies": map[string]any{"invalid": "not-a-list"}, + }, + "child": {"extends": ".base", "stage": "build"}, + }) + _, err := Resolve(p) + if err == nil { + t.Fatal("expected error when yaml.Unmarshal fails on incompatible type") + } + }) } diff --git a/internal/resolver/includes.go b/internal/resolver/includes.go index 354b08d..6e81dcf 100644 --- a/internal/resolver/includes.go +++ b/internal/resolver/includes.go @@ -305,9 +305,6 @@ func substituteInputs(data []byte, inputs map[string]any) []byte { } return inputPlaceholderRe.ReplaceAllFunc(data, func(match []byte) []byte { groups := inputPlaceholderRe.FindSubmatch(match) - if len(groups) < 2 { - return match - } if val, ok := inputs[string(groups[1])]; ok { return []byte(fmt.Sprintf("%v", val)) } diff --git a/internal/resolver/includes_test.go b/internal/resolver/includes_test.go index 90c0c5a..4646ba6 100644 --- a/internal/resolver/includes_test.go +++ b/internal/resolver/includes_test.go @@ -644,6 +644,35 @@ func TestResolveProjectInclude_InvalidYAML(t *testing.T) { } } +// TestResolveProjectInclude_WithSubIncludes covers includes.go:236-240 — +// the fetched project YAML itself contains include: entries. +func TestResolveProjectInclude_WithSubIncludes(t *testing.T) { + dir := t.TempDir() + // Write a local file that the project YAML sub-includes. + localContent := "local-from-project:\n script: echo local\n" + if err := os.WriteFile(filepath.Join(dir, "local.yml"), []byte(localContent), 0o644); err != nil { + t.Fatal(err) + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + // Return YAML that itself has a local sub-include. + fmt.Fprintln(w, "include:\n - local: /local.yml\nproj-sub-job:\n script: echo proj") + })) + defer srv.Close() + + p := &model.Pipeline{Jobs: map[string]model.Job{}, RawJobs: map[string]map[string]any{}} + cfg := fetcher.GitLabConfig{BaseURL: srv.URL, Token: "tok"} + entry := map[string]any{"project": "g/p", "file": "ci.yml", "ref": "main"} + warnings, _ := resolveProjectInclude(p, entry, "g/p", cfg, dir, map[string]bool{}, 0) + if len(warnings) != 0 { + t.Errorf("sub-includes: unexpected warnings: %v", warnings) + } + if _, ok := p.Jobs["proj-sub-job"]; !ok { + t.Error("proj-sub-job should be merged via project include") + } +} + // tlsComp sets up a TLS test server and swaps http.DefaultTransport so the test // client trusts the self-signed cert. Returns the host (without scheme). func tlsComp(t *testing.T, h http.HandlerFunc) (host string) { @@ -691,6 +720,33 @@ func TestResolveComponentInclude_InvalidYAML(t *testing.T) { } } +// TestResolveComponentInclude_WithSubIncludes covers includes.go:288-291 — +// the fetched component YAML itself contains include: entries. +func TestResolveComponentInclude_WithSubIncludes(t *testing.T) { + dir := t.TempDir() + localContent := "comp-local-job:\n script: echo comp-local\n" + if err := os.WriteFile(filepath.Join(dir, "sub.yml"), []byte(localContent), 0o644); err != nil { + t.Fatal(err) + } + + host := tlsComp(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + // Component YAML contains a local sub-include. + fmt.Fprintln(w, "include:\n - local: /sub.yml\ncomp-job:\n script: echo comp") + }) + ref := host + "/g/p/mycomp@v1" + + p := &model.Pipeline{Jobs: map[string]model.Job{}, RawJobs: map[string]map[string]any{}} + cfg := fetcher.GitLabConfig{} + _, extW, hadErr := resolveComponentInclude(p, ref, nil, cfg, dir, map[string]bool{}, 0) + if hadErr { + t.Errorf("sub-includes: unexpected hadErr; extWarnings=%v", extW) + } + if _, ok := p.Jobs["comp-job"]; !ok { + t.Error("comp-job should be merged via component include") + } +} + // ── resolveRemoteInclude — sub-includes ─────────────────────────────────────── func TestResolveRemoteInclude_WithSubIncludes(t *testing.T) {