-
Notifications
You must be signed in to change notification settings - Fork 20
ci: gate openjd-cli on the OpenJD conformance suite #234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mainline
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
|
|
||
| jobs: | ||
| conformance: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No This job only needs to read code, and it goes on to execute a test runner from another repository (see the permissions:
contents: readDeclaring it at the top level (or on the |
||
| name: Conformance (${{ matrix.os }}) | ||
| runs-on: ${{ matrix.os }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Operational note: this job has no concurrency:
group: conformance-${{ github.ref }}
cancel-in-progress: true
jobs:
conformance:
timeout-minutes: 30 # tune to observed runtimeThe |
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest, windows-latest, macos-latest] | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v7 | ||
| with: | ||
| ref: ${{ inputs.tag }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Nothing in this job pushes, so the credential is not needed: - name: Checkout
uses: actions/checkout@v7
with:
ref: ${{ inputs.tag }}
persist-credentials: falseThis 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Not a bug for the stated need — the fixtures run |
||
| # 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This checks out
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
|
||
| - 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/*' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the external suite really does emit |
||
| not a crash. | ||
|
|
||
| ## Things to Know | ||
|
|
||
| ### The Package's Public Interface | ||
|
|
||
| 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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 That undercuts the stated purpose. The template description says "losing grandchild output [...] silently weakens that whole suite", and 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 |
||
| 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) | ||
| 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) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( It is also largely redundant with |
||
|
|
||
|
|
||
| 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) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
workflow_calltrigger and itstag/branchinputs are never exercised — no workflow in the repo calls./.github/workflows/conformance.yml. Comparecode_quality.yml, which declares the same input surface and is invoked fromrelease_publish.yml:33as theUnitTestsjob.So the
ref: ${{ inputs.tag }}plumbing below is currently dead: onpull_requestthe expression evaluates to the empty string andactions/checkoutfalls back to the event ref (correct behaviour, just not what the input is for). Either wire conformance into the release pipeline alongsideUnitTests, or drop theworkflow_callblock and theref:until it is needed — otherwise it reads as configured-and-working when it has never run.Separately,
branchis declared but unused even in the intended design (code_quality.ymlhas the same vestigial input, so this may just be copied).