Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions .github/workflows/conformance.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
name: Conformance

on:
pull_request:
branches: [ mainline, release, 'patch_*' ]
workflow_call:
inputs:
branch:
required: false
type: string
tag:
required: false
type: string

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow_call trigger and its tag / branch inputs are never exercised — no workflow in the repo calls ./.github/workflows/conformance.yml. Compare code_quality.yml, which declares the same input surface and is invoked from release_publish.yml:33 as the UnitTests job.

So the ref: ${{ inputs.tag }} plumbing below is currently dead: on pull_request the expression evaluates to the empty string and actions/checkout falls back to the event ref (correct behaviour, just not what the input is for). Either wire conformance into the release pipeline alongside UnitTests, or drop the workflow_call block and the ref: until it is needed — otherwise it reads as configured-and-working when it has never run.

Separately, branch is declared but unused even in the intended design (code_quality.yml has the same vestigial input, so this may just be copied).


jobs:
conformance:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No permissions: block, so this job inherits the repository/org default GITHUB_TOKEN scopes rather than least privilege. Every other workflow added to this repo declares them explicitly (permissions: {} in claude_pr_review_collect.yml, contents: read in release_publish.yml, security-events: write in codeql.yml).

This job only needs to read code, and it goes on to execute a test runner from another repository (see the openjd-specifications checkout below), so leaving the token broader than necessary is worth avoiding:

permissions:
  contents: read

Declaring it at the top level (or on the conformance job) keeps it consistent with the rest of .github/workflows/.

name: Conformance (${{ matrix.os }})
runs-on: ${{ matrix.os }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational note: this job has no timeout-minutes and the workflow has no concurrency group. It runs the full conformance suite across three OSes, and the suite launches job fixtures that spawn subprocesses — a fixture that hangs will hold a runner for the default 6-hour limit, ×3.

concurrency:
  group: conformance-${{ github.ref }}
  cancel-in-progress: true

jobs:
  conformance:
    timeout-minutes: 30   # tune to observed runtime

The concurrency block also stops rapid pushes to a PR from stacking up three-OS runs; claude_pr_review.yml already uses this pattern in the repo.

strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
steps:
- name: Checkout
uses: actions/checkout@v7
with:
ref: ${{ inputs.tag }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actions/checkout defaults to persist-credentials: true, which leaves the GITHUB_TOKEN in .git/config of this checkout for the life of the job. The last step then executes code from a different repository (uv run run_openjd_cli_tests.py) in the same workspace, and that suite in turn launches job fixtures that run shell/python commands. Any of that code can read $GITHUB_WORKSPACE/.git/config and use the token.

Nothing in this job pushes, so the credential is not needed:

      - name: Checkout
        uses: actions/checkout@v7
        with:
          ref: ${{ inputs.tag }}
          persist-credentials: false

This matters more here than in a typical build job precisely because the job’s purpose is to run externally-authored fixtures — and it compounds with the inherited token scopes noted above.

- uses: actions/setup-python@v7
with:
python-version: '3.12'
# The suite's job fixtures run `command: python`, which does not exist on a
# bare Ubuntu runner -- only `python3` does. A virtualenv provides both names

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment claims "A virtualenv provides both names" (python and python3), but that is not true on Windows: CPython does not create a python3.exe in Scripts/ — only python.exe and pythonw.exe. A symlinked python3 exists only in POSIX venvs (bin/python, bin/python3, bin/pythonX.Y).

Not a bug for the stated need — the fixtures run command: python, which the Windows venv does provide — but if any fixture ever uses python3 the Windows leg will fail while Linux/macOS pass, and this comment would send the reader looking in the wrong place. Worth narrowing the claim to what is actually relied on (the venv provides python plus the openjd entry point) rather than asserting it covers all three names on every OS.

# plus the `openjd` entry point the runner invokes, so putting its bin
# directory on PATH covers all three.
- name: Create virtualenv
shell: bash
run: |
# Outside the checkout, so it cannot end up in a source distribution.
python -m venv "$RUNNER_TEMP/conformance-venv"
if [ -d "$RUNNER_TEMP/conformance-venv/Scripts" ]; then
echo "$RUNNER_TEMP/conformance-venv/Scripts" >> "$GITHUB_PATH"
else
echo "$RUNNER_TEMP/conformance-venv/bin" >> "$GITHUB_PATH"
fi
- name: Install openjd-cli
shell: bash
run: |
python -m pip install --upgrade pip
python -m pip install .
- name: Checkout openjd-specifications
uses: actions/checkout@v7
with:
repository: OpenJobDescription/openjd-specifications
path: openjd-specifications

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks out openjd-specifications at whatever its default branch happens to point at, and the next steps execute code from it (uv run run_openjd_cli_tests.py, plus whatever dependencies uv resolves for that script). Two consequences:

  1. Reproducibility — a push to openjd-specifications mainline can turn this PR check red without anything changing in this repo, and a previously-green commit will not necessarily re-run green.
  2. Trust — the job runs arbitrary code from a floating ref of another repo. That is a sibling repo in the same org so the risk is modest, but combined with the inherited GITHUB_TOKEN scopes (see the earlier comment) it is worth constraining.

Consider pinning to a tag or commit SHA and bumping it deliberately, e.g.

      - name: Checkout openjd-specifications
        uses: actions/checkout@v7
        with:
          repository: OpenJobDescription/openjd-specifications
          ref: <tag-or-sha>
          path: openjd-specifications
          persist-credentials: false

persist-credentials: false is also worth adding here since nothing in the job pushes to that checkout.

- name: Install uv
uses: astral-sh/setup-uv@v9.0.0
- name: Run conformance tests
shell: bash
working-directory: openjd-specifications/conformance-tests
run: |
openjd --version
uv run run_openjd_cli_tests.py '2023-09/*'
28 changes: 28 additions & 0 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,34 @@ log_cli_level = 10
```
3. Add logging statements to your tests as desired and run the test(s) that you are debugging.

### Running the conformance suite

The [openjd-specifications](https://github.com/OpenJobDescription/openjd-specifications) conformance
suite checks this CLI's behaviour against the specification. It drives the installed `openjd`
command, so it exercises the entry point rather than importing the package. CI runs it on Linux,
macOS and Windows via `.github/workflows/conformance.yml`.

To run it locally you need a checkout of `openjd-specifications`, [uv](https://docs.astral.sh/uv/),
and `openjd` on `PATH`. `hatch shell` provides the latter:

```bash
hatch shell
cd /path/to/openjd-specifications/conformance-tests
uv run run_openjd_cli_tests.py '2023-09/*' # the whole suite
uv run run_openjd_cli_tests.py '2023-09/base/jobs' # one directory
uv run run_openjd_cli_tests.py '2023-09/base/jobs/1.1--basic-job-creation.test.yaml'
```

The job fixtures run `command: python`, so `python` — not just `python3` — has to resolve on
`PATH`. A virtualenv or `hatch shell` satisfies this; a bare system Python on Ubuntu does not.

Two failure shapes are worth telling apart. A template test failing means `openjd check` accepted
something it should have rejected, or vice versa. A job test failing means `openjd run` produced the
wrong output, or exited non-zero when the fixture expected a clean run — many of the single-task job
fixtures assert their own output from inside the task and signal a mismatch through the task's exit
status, so a non-zero exit with `OPENJD_CONFORMANCE_ASSERT_FAILED` in the log is an output mismatch,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OPENJD_CONFORMANCE_ASSERT_FAILED does not appear anywhere in this repo, and the fixture added in this same PR emits a different string — ASSERT_FAILED: expected output line not found (test/openjd/cli/templates/self_asserting_task.yaml:39), which is what the new tests grep for.

If the external suite really does emit OPENJD_CONFORMANCE_ASSERT_FAILED, this is fine but worth double-checking against the current openjd-specifications fixtures, since a reader following this doc will grep the log for a string that may not be there. If it was extrapolated from the local fixture, the doc and the fixture should use the same marker so the instruction is actually actionable.

not a crash.

## Things to Know

### The Package's Public Interface
Expand Down
40 changes: 40 additions & 0 deletions test/openjd/cli/templates/self_asserting_task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
specificationVersion: "jobtemplate-2023-09"
name: Self Asserting Task
description: >
An onRun action that spawns its own child process, reproduces that child's
output, and turns a comparison of it into the task's exit status. This is the
shape the openjd-specifications conformance suite uses for its single-task job
fixtures, so losing grandchild output or dropping the action's exit status
silently weakens that whole suite.

parameterDefinitions:
- name: Printed
type: STRING
default: "EXPECTED_VALUE"

steps:
- name: Assert
script:
actions:
onRun:
command: python
args: ["{{Task.File.Assert}}"]
embeddedFiles:
- name: Assert
type: TEXT
data: |
import subprocess
import sys

COMMAND = [sys.executable, "-c", "print(r'OUTPUT:{{Param.Printed}}')"]
EXPECTED = ["OUTPUT:EXPECTED_VALUE"]

completed = subprocess.run(COMMAND, capture_output=True, text=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capture_output=True means the grandchild never writes to the action’s inherited stdout/stderr at all — the pipes are drained into completed.stdout/completed.stderr and the action then re-emits them through its own streams on lines 33-34.

So the runner only ever sees output produced directly by the process it spawned. If the CLI/session dropped grandchild output entirely, both new tests would still pass, because the bytes reach the log via the action’s own sys.stdout.write.

That undercuts the stated purpose. The template description says "losing grandchild output [...] silently weakens that whole suite", and test_self_asserting_task.py:17 is named test_grandchild_output_captured_and_assertion_passes with the comment "Missing means output from a process we did not spawn ourselves was dropped" — but nothing here exercises that path.

To actually pin grandchild output propagation the child has to inherit the streams rather than have them captured, e.g. print a second marker without capturing:

subprocess.run([sys.executable, "-c", "print(\"INHERITED:{{Param.Printed}}\")"])

and assert INHERITED:EXPECTED_VALUE shows up. As written, the fixture is a valid test of action exit-status propagation (which the second test does cover), just not of grandchild output capture.

sys.stdout.write(completed.stdout)
sys.stderr.write(completed.stderr)
output = completed.stdout + completed.stderr

missing = [line for line in EXPECTED if line not in output]
for _ in missing:
sys.stderr.write("ASSERT_FAILED: expected output line not found\n")
sys.exit(1 if missing else 0)
42 changes: 42 additions & 0 deletions test/openjd/cli/test_self_asserting_task.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
"""The openjd-specifications conformance shape where a task asserts its own output.

Its single-task job fixtures spawn the case's command as a child, reproduce its
output, and exit non-zero when the output does not match. That verdict only reaches
the runner if we capture a grandchild's output and propagate the action's exit status,
so both directions are pinned here rather than left to the external suite to catch.
"""

from pathlib import Path

from . import format_capsys_outerr, run_openjd_cli_main

TEMPLATE_DIR = Path(__file__).parent / "templates"


def test_grandchild_output_captured_and_assertion_passes(capsys):
outerr = run_openjd_cli_main(
capsys,
args=["run", str(TEMPLATE_DIR / "self_asserting_task.yaml")],
expected_exit_code=0,
)
# Printed by the grandchild and echoed by the action. Missing means output from a
# process we did not spawn ourselves was dropped.
assert "OUTPUT:EXPECTED_VALUE" in outerr.out, format_capsys_outerr(outerr)
assert "ASSERT_FAILED" not in outerr.out, format_capsys_outerr(outerr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This negative assertion can never fire. The fixture writes the marker to stderr (sys.stderr.write("ASSERT_FAILED: ..."), self_asserting_task.yaml:39), but this checks outerr.out only. Note the sibling test on line 40 correctly checks outerr.out + outerr.err.

It is also largely redundant with expected_exit_code=0: the fixture only emits the marker on the same branch where it exits 1, so a run that reaches this line already cannot have failed the assertion. Either check both streams for symmetry with the other test, or drop the line.



def test_assertion_failure_fails_the_run(capsys):
outerr = run_openjd_cli_main(
capsys,
args=[
"run",
str(TEMPLATE_DIR / "self_asserting_task.yaml"),
"-p",
"Printed=WRONG_VALUE",
],
expected_exit_code=1,
)
output = outerr.out + outerr.err
assert "ASSERT_FAILED" in output, format_capsys_outerr(outerr)
assert "OUTPUT:WRONG_VALUE" in output, format_capsys_outerr(outerr)
Loading