Add YAML loader validation tests for workflow contracts

[coderabbitai]

Summary

Add focused unit tests for the YAML workflow loader validation added in tests/workflow_contracts/ci_lint_test.py.

Rationale

The current change adds YAML 1.2 boolean handling and validates documents returned by _load. Existing tests do not cover these branches. Add direct coverage to prevent regressions in GitHub Actions workflow parsing and validation.

Affected area

• tests/workflow_contracts/ci_lint_test.py
• Its associated Python test module

Required changes

Add focused tests for _WorkflowLoader and _load.

Acceptance criteria

• A workflow document keeps the on key as a string.
• YAML true and false values resolve as booleans.
• _load rejects an empty document.
• _load rejects scalar documents.
• _load rejects list documents.
• _load rejects mappings that contain non-string keys.
• The tests assert the intended error behaviour without depending on unrelated workflow contract tests.

Backlinks

• Follow-up from PR Address PR #515 review follow-ups #530: Address PR #515 review follow-ups #530
• Originating review comment: Address PR #515 review follow-ups #530 (comment)
• Requested by: @leynos

[coderabbitai]

🔗 Related PRs

leynos/agent-template-rust#52 - Add scheduled mutation testing to generated repos [merged]
leynos/lag-complexity#72 - Wire CodeScene coverage upload on main and prepare the PR gate [merged]
leynos/stilyagi#67 - Onboard CodeScene coverage generation and main-branch upload [merged]
leynos/diesel-cte-ext#111 - Document mutation-testing workflow contract tests [open]
#473 - Adopt cargo-nextest as the default test runner (#463) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.\n\nOnce a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh issue view 543 \
  --repo leynos/netsuke \
  --json number,title,body,state,labels,url,comments

printf '\n--- repository labels ---\n'
gh label list --repo leynos/netsuke --limit 200

Length of output: 6610

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- candidate files ---'
fd -t f . | rg '(^|/)(ci_lint_test|.*ci_lint.*test|.*workflow.*contract.*)\.py$|(^|/)(README|CONTRIBUTING|docs|.*workflow.*)\.(md|rst|adoc|ya?ml)$' || true

printf '%s\n' '--- Python module outlines ---'
fd -t f -e py . tests | rg 'ci_lint|workflow' | while IFS= read -r file; do
  printf '\n### %s\n' "$file"
  ast-grep outline "$file" --items all --type class,function --view compact || true
done

printf '%s\n' '--- Loader, load, and focused-test references ---'
rg -n -i -C 5 \
  'class\s+_WorkflowLoader\b|def\s+_load\b|_WorkflowLoader\b|_load\s*\(|YAML\s*1\.2|non-string\s+key|empty\s+document|scalar\s+document' \
  -g '*.py' -g '*.md' -g '*.rst' -g '*.adoc' -g '*.yaml' -g '*.yml' .

printf '%s\n' '--- Documentation references ---'
rg -n -i -C 3 \
  'workflow contract|workflow loader|yaml|GitHub Actions|ci_lint' \
  -g '*.md' -g '*.rst' -g '*.adoc' -g '*.txt' .

printf '%s\n' '--- Relevant history ---'
git log --oneline --all --decorate -- \
  tests/workflow_contracts/ci_lint_test.py \
  tests \
  | head -80

Length of output: 201592

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- workflow-contract files ---'
fd -t f -e py . tests/workflow_contracts | sort

printf '%s\n' '--- loader source ---'
nl -ba tests/workflow_contracts/ci_lint_test.py | sed -n '1,260p'

printf '%s\n' '--- direct loader test candidates ---'
rg -n -l -i \
  '_WorkflowLoader|ci_lint_test|empty document|scalar document|non-string key|YAML 1\.2|on key' \
  -g '*.py' tests \
  | sort \
  | while IFS= read -r file; do
      printf '\n### %s\n' "$file"
      nl -ba "$file" | sed -n '1,340p'
    done

printf '%s\n' '--- focused documentation references ---'
rg -n -C 4 \
  'ci_lint_test\.py|workflow contract test|workflow loader' \
  -g '*.md' -g '*.rst' -g '*.adoc' \
  README.md docs 2>/dev/null || true

printf '%s\n' '--- commits that changed the loader or direct tests ---'
git log --all --format='%H%x09%ad%x09%s' --date=short -- \
  tests/workflow_contracts/ci_lint_test.py \
  tests/workflow_contracts \
  | head -40

Length of output: 1761

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- ci_lint_test.py ---'
sed -n '1,360p' tests/workflow_contracts/ci_lint_test.py

printf '%s\n' '--- all direct loader references in Python tests ---'
rg -n -i -C 6 \
  '_WorkflowLoader|def _load|_load\(|empty document|scalar document|non-string key|YAML 1\.2|on key' \
  -g '*.py' tests/workflow_contracts tests \
  || true

printf '%s\n' '--- documentation references to this test contract ---'
rg -n -i -C 5 \
  'tests/workflow_contracts/ci_lint_test\.py|workflow loader|workflow contract test' \
  -g '*.md' -g '*.rst' -g '*.adoc' \
  README.md docs 2>/dev/null || true

printf '%s\n' '--- relevant history ---'
git log --all --format='%H%x09%ad%x09%s' --date=short -- \
  tests/workflow_contracts/ci_lint_test.py \
  tests/workflow_contracts \
  | head -40

Length of output: 80520

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh issue edit 543 \
  --repo leynos/netsuke \
  --add-label low

gh issue view 543 \
  --repo leynos/netsuke \
  --json labels,url \
  --jq '{url, labels: [.labels[].name]}'

Length of output: 484

---

@leynos, keep this issue open.

Status: The issue remains relevant.

tests/workflow_contracts/ci_lint_test.py implements the required loader behaviour. _WorkflowLoader preserves on as a string and resolves true and false as booleans. _load rejects empty, scalar, list, and non-string-keyed documents.

The file does not contain focused tests for these behaviours. The current tests only load the checked-in workflows. They do not exercise invalid YAML documents or directly assert the YAML 1.2 resolver contract.

Documentation: No documentation update is required. This issue changes test coverage only. It does not change the documented workflow format, contributor procedure, or Netsuke user behaviour. Do not add the documentation label.

Criticality: low.

The missing tests can allow a regression in workflow-contract test parsing. The effect is limited to CI test infrastructure. It does not affect the Netsuke runtime, release artefacts, or user-facing APIs. The change scope is one Python test module and temporary YAML fixtures.

Impact of required changes: Add direct unit tests for _WorkflowLoader and _load in tests/workflow_contracts/ci_lint_test.py or its associated test module. Cover the six acceptance criteria from the issue. The change has a small blast radius and should not require production-code or documentation changes.

Applied the low label. The issue labels are now enhancement, testing, and low.

_{You are interacting with an AI system.}

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

Add the new tests inside ci_lint_test.py, beside the private helpers they exercise, because no __all__ or cross-module import pattern exists.

Test _WorkflowLoader by parsing small synthetic snippets directly, asserting trigger words stay strings and literal booleans resolve to bool.

Test the _load rejection branches with tmp_path files and pytest.raises(pytest.fail.Exception, match=...), and document this new idiom with a comment.

Keep the tests self-contained on synthetic input, so they never depend on the real workflow files or on other contract tests.

Design Choices

Design Choice 1: Where do the new tests belong?

Options Considered:

1. Add the new tests as functions inside ci_lint_test.py, next to the helpers they exercise.
2. Create a new companion module and import the private helpers from ci_lint_test.py.

Chosen Option: 1

Rationale: The helpers _load and _WorkflowLoader are module-private with no __all__, and the repository never imports names across *_test.py files (shared state travels through conftest.py instead). Option 1 needs no cross-module import of private names, keeps the tests beside the code under test, and matches the self-contained convention of every contract-test module.

Design Choice 2: How do the tests assert the `_load` rejection branches?

Options Considered:

1. Wrap the _load call in pytest.raises(pytest.fail.Exception, match=...) and assert the failure message.
2. Refactor _load to raise a domain exception, then assert that exception.

Chosen Option: 1

Rationale: _load reports structural violations with pytest.fail(...), which raises pytest.fail.Exception. The repository has no precedent for catching this outcome, and _load must not be refactored beyond the ticket scope. pytest.fail.Exception is public pytest API, this needs no production change, and it directly asserts the intended failure text. Add a short comment because the idiom is new to this repository.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Boolean resolution tests for _WorkflowLoader

This phase proves that the custom resolver keeps GitHub Actions trigger words as strings and still resolves literal booleans. The tests parse small synthetic snippets with _WorkflowLoader directly, so they do not depend on the real workflow files or on other contract tests.

Task 1: Assert trigger words stay strings

Add tests that confirm the loader does not coerce YAML 1.1 boolean words.

• Add test functions to tests/workflow_contracts/ci_lint_test.py that call yaml.load(<snippet>, Loader=_WorkflowLoader) on minimal mappings.
• Assert that on, yes, off, no parse to the strings "on", "yes", "off", "no", not to Python booleans.
• Assert that the on: key of a mapping stays the string key "on" and does not become True.
• Give each assertion a descriptive message that states why the string form matters for the on: trigger.

Task 2: Assert literal booleans still resolve

Add a test that confirms YAML 1.2 boolean literals resolve to Python bool.

• Add a test function that parses true, True, TRUE, false, False, FALSE values with _WorkflowLoader.
• Assert the parsed values are Python True/False.
• Use @pytest.mark.parametrize with pytest.param(..., id=...) entries for the six case variants, following the parametrise style in scripts/tests/test_typos_rollout.py.

🤖 Prompt for AI agents

Implement Phase 1: add unit tests that prove `_WorkflowLoader` resolves booleans
correctly against synthetic YAML snippets, with no dependency on real workflow
files.

For trigger-word tests:
- Add test functions to `tests/workflow_contracts/ci_lint_test.py` that call
`yaml.load(<snippet>, Loader=_WorkflowLoader)` on minimal mapping snippets.
- Assert that `on`, `yes`, `off`, `no` parse to the strings `"on"`, `"yes"`,
`"off"`, `"no"` and never to Python booleans.
- Assert that the `on:` key of a mapping stays the string key `"on"` and does
not become `True`.
- Give each assertion a descriptive failure message that states why the string
form matters for the `on:` trigger.

For literal-boolean tests:
- Add a test function that parses `true`, `True`, `TRUE`, `false`, `False`,
`FALSE` values with `_WorkflowLoader`.
- Assert the parsed values equal Python `True`/`False`.
- Use `@pytest.mark.parametrize` with `pytest.param(..., id=...)` entries for
each of the six case variants.
- Follow the parametrise style used in `scripts/tests/test_typos_rollout.py`.

Research

The repository holds no pyproject.toml, pytest.ini, or setup.cfg. Pytest runs on default discovery and matches both test_*.py and *_test.py. The target file tests/workflow_contracts/ci_lint_test.py defines the module-private helpers _WorkflowLoader, _is_string, and _load. _WorkflowLoader subclasses yaml.SafeLoader and rebinds yaml_implicit_resolvers so that on, yes, and off stay strings, while true/false (and the True/TRUE/False/FALSE case variants) resolve to Python booleans. _load reads a file path, parses it with _WorkflowLoader, and calls pytest.fail(...) for every structural violation (non-mapping root, non-string keys, bad jobs, bad env, redeclared NEXTEST_VERSION). No test currently exercises these branches with synthetic input. The module has no __all__, and no sibling *_test.py file imports from it. The Makefile target test-workflow-contracts runs pytest tests/workflow_contracts -q with pytest, pyyaml, and hypothesis available, and collects new tests automatically.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Implement Phase 1: add unit tests that prove `_WorkflowLoader` resolves booleans
correctly against synthetic YAML snippets, with no dependency on real workflow
files.

For trigger-word tests:
- Add test functions to `tests/workflow_contracts/ci_lint_test.py` that call
`yaml.load(<snippet>, Loader=_WorkflowLoader)` on minimal mapping snippets.
- Assert that `on`, `yes`, `off`, `no` parse to the strings `"on"`, `"yes"`,
`"off"`, `"no"` and never to Python booleans.
- Assert that the `on:` key of a mapping stays the string key `"on"` and does
not become `True`.
- Give each assertion a descriptive failure message that states why the string
form matters for the `on:` trigger.

For literal-boolean tests:
- Add a test function that parses `true`, `True`, `TRUE`, `false`, `False`,
`FALSE` values with `_WorkflowLoader`.
- Assert the parsed values equal Python `True`/`False`.
- Use `@pytest.mark.parametrize` with `pytest.param(..., id=...)` entries for
each of the six case variants.
- Follow the parametrise style used in `scripts/tests/test_typos_rollout.py`.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---