Repository navigation
packaging(ns8): wire up the robot suite - #144
Conversation
Splits tests/calnode.robot into the NS8 file-naming convention so ordering is explicit and the [ui] test can be skipped on its own, and adds a workflow to run the suite in CI. Dry-run clean: 5/5 under Robot 7.5, order verified as install -> configure -> route -> screenshots -> remove, and --exclude ui correctly drops to 4. Do not reach for the stock NethServer/ns8-github-actions test-module.yml@v1 here. It derives the image under test from the repository name (basename, minus an ns8- prefix) and forces main to :latest, which for this repo resolves to ghcr.io/calnode/calnode:latest. That is the app image, which carries no NS8 module manifest, so add-module would fail; and this repo deliberately never publishes a :latest module tag, since :latest stays pinned to releases. So this calls test-on-digitalocean-infra.yml@v1 directly, which is the reusable workflow test-module.yml delegates to anyway, and supplies the image itself. The workflow is manual-only: it needs a do_token secret this repo does not have, and a workflow_run trigger today would just fail red on every publish. Both the missing trigger and the image reasoning are documented in the header. Also git-ignores tests/outputs/, which robot writes output.xml, log.html and report.html into, and adds a README Testing section.
There was a problem hiding this comment.
Important
The wiring itself checks out — I dry-ran the split suite (5/5 with the Playwright image, 4/4 under --exclude ui, ${module_id} reaching 20__remove.robot) and traced the runner contract against ns8-github-actions@v1. Two things in the workflow header need correcting before anyone leans on them: the workflow_run block this file tells the next person to uncomment does not work as written, and the "never publishes a :latest module tag" claim is contradicted by ns8-module.yml.
Reviewed changes
- Robot suite split into the NS8 file-naming convention —
calnode.robotreplaced by10__calnode.robot(install, configure, route responds),15__ui.robot(Status/Settings screenshots, taggedui) and20__remove.robot(removal), preserving the__init__.robotconnection setup, the${module_id}global, and the 5-test / 4-test--exclude uibehaviour. - Hand-rolled robot workflow —
.github/workflows/test-module.ymlcallstest-on-digitalocean-infra.yml@v1directly instead oftest-module.yml@v1, passingpath,args(the module image),leader_nodes: rl1,run_ui_testsanddebug_shell;workflow_dispatch-only pending ado_tokensecret, with a long header explaining why the stock entry point is bypassed. - Runner output ignored —
.gitignoreaddspackaging/ns8/tests/outputs/, which is where the runner writesoutput.xml,log.html,report.htmland the screenshots. - README — new Testing section with the per-file table, the local
run-ns8-testsinvocation and the CI note; the live-testing caveat now says dry-run clean but never executed against a cluster.
ℹ️ The suite never exercises update, and the secret-free runner is not mentioned
Two things this leaves on the table, neither of which needs blocking the PR, but both worth a line in the Testing section so the coverage is not over-claimed later.
Affected sites
.github/workflows/test-module.yml:74-82— noinstall/updatematrix and no-v SCENARIO:...; upstream's stocktest-module.ymlruns both legs and passesSCENARIO, which10__calnode.robotnever reads.update-moduleis therefore never exercised..github/workflows/ns8-module.ymlpublishes module tags onvX.Y.Zpushes, and the NS8 module contract explicitly supports upgrade paths, so the leg that matters most for a released module is the one with no coverage.- Upstream also ships
.github/workflows/test-module-qemu.yml(seedocs/test-module-qemu.md), which declaresdo_tokenasrequired: falseand runs the same suite under QEMU with no DigitalOcean token and no droplets.packaging/ns8/README.md:197-199frames manual-only as a consequence of the missing secret; that is true of this workflow, but not the only way to get automated coverage today.
Open questions for the human
- Is the missing
updateleg deliberate for now, or an oversight? Upstream's QEMU runner drops theupdatescenario automatically whentests/never reads${SCENARIO}, so a plainSCENARIO-free suite is an expected shape there — but this repo's module does ship release tags, so it is worth a conscious decision rather than silence.
space-bunny-free (free) | 𝕏
| # * the module image is ghcr.io/calnode/ns8-calnode, not ghcr.io/calnode/calnode | ||
| # (the latter is the Calnode application image, which has no NS8 module | ||
| # manifest, so add-module would fail); | ||
| # * this repo deliberately never publishes a :latest module tag. :latest stays |
There was a problem hiding this comment.
This claim is false, and the very next clause contradicts it: ns8-module.yml:118-121 publishes module latest on a vX.Y.Z tag (module_tags=${version} ${version%.*} latest), and push-manifest.sh pushes every entry in IMAGETAGS. Only main publishes edge, which is what README.md:149-151 already says accurately. The conclusion survives — the stock workflow resolves the name to calnode, not ns8-calnode, which alone settles it — but this header exists so nobody re-derives the decision, and a header whose second reason expires at the first module release is the kind of thing that gets someone to "fix" the workflow back.
Technical details
# The ":latest is never published" justification is wrong
## Affected sites
- `.github/workflows/test-module.yml:23-25` — asserts the repo "deliberately never publishes a :latest module tag", then says ":latest stays pinned to releases" in the same bullet.
- `.github/workflows/ns8-module.yml:118-121` — on `refs/tags/v*`, `module_tags=${version} ${version%.*} latest`.
- `packaging/ns8/push-manifest.sh:51-72` — iterates `IMAGETAGS` and pushes each assembled manifest, so `latest` really lands.
- `packaging/ns8/README.md:149-151` — states the accurate version: "no module release has been cut yet".
## Required outcome
- Drop the second bullet, or restate it as what is actually true: `main` publishes `:edge`, and `:latest` only moves on a release tag. The first bullet (wrong image *name*) is by itself sufficient justification and does not expire.
## Suggested approach
- Replace lines 23-25 with something like: `:latest moves only on a release tag; main publishes :edge, mirroring docker-publish.yml.`
- Consider also correcting the same claim in the PR description, which repeats it.| # workflow_run: | ||
| # workflows: ['NS8 module publish'] |
There was a problem hiding this comment.
Uncommenting this as written produces a red, billable run. Under workflow_run the inputs context is empty and default: is not applied, so args: passes nothing — and that second positional argument is exactly what selects module mode in test-module.sh:20-28, which then never defines IMAGE_URL and test 1 dies on add-module ${IMAGE_URL}. Two more fall out of the same edit: run_ui_tests/debug_shell become empty strings where upstream declares type: boolean, and repo_ref falls back to github.sha (the default-branch tip, not the commit that was published), so the checkout would not match the image under test.
Technical details
# The documented `workflow_run` follow-up is not self-contained
## Affected sites
- `.github/workflows/test-module.yml:57-61` — the commented trigger block.
- `.github/workflows/test-module.yml:77` — `args: ${{ inputs.image }}`, empty under `workflow_run`.
- `.github/workflows/test-module.yml:81-82` — the two `type: boolean` inputs, empty under `workflow_run`.
- `packaging/ns8/tests/10__calnode.robot:10` — `add-module ${IMAGE_URL}`, undefined in core mode.
- Upstream `scripts/test-module.sh:20-28` — `if [ -n "${2:-}" ] && [[ $2 != -* ]]; then IMAGE_URL="$2"; mode="module"` … `else mode="core"`.
- Upstream `scripts/test-module.sh:137-139` — `-v "IMAGE_URL:${IMAGE_URL}"` is added only in module mode.
- Upstream `test-on-digitalocean-infra.yml:19-23,58-62` — `repo_ref` defaults to `${{ github.sha }}`; `run_ui_tests`/`debug_shell` are `type: boolean`.
- Upstream `test-module.yml:22,66` — the stock path guards with `if: github.event.workflow_run.conclusion == 'success'` and threads `repo_ref: ${{ needs.module.outputs.sha }}` (= `github.event.workflow_run.head_sha || github.sha`).
## Required outcome
- Whatever the trigger becomes, the run must (a) only fire on a successful publish, (b) derive the image URL and the tested ref from `github.event.workflow_run.*` rather than from `inputs`, since `inputs` does not exist for that event.
## Suggested approach
- Add the success guard: `if: github.event.workflow_run.conclusion == 'success'` on the `test` job.
- Give each `with:` value a fallback that works for both events, e.g. `args: ${{ inputs.image || format('ghcr.io/calnode/ns8-calnode:{0}', ...) }}` or, more simply, compute a local `env:` block that prefers `inputs.*` and falls back to the workflow_run ref.
- Set `repo_ref: ${{ github.event.workflow_run.head_sha || github.sha }}`.
- Note that `types: [completed]` also fires for the `pull_request` runs of "NS8 module publish", which build but push nothing — those would test the previous `edge` image against the PR's test files.
## Open questions for the human
- Is `workflow_run` the intended trigger at all, or should this become `workflow_call` so the `NS8 module publish` workflow calls it directly with an explicit image? That sidesteps every `inputs` problem above.
Gets the NS8 robot suite ready to actually run. It still has never executed against a live cluster, and this PR is explicit about that rather than implying otherwise.
What changed
Suite split into NS8 file-naming convention
10__calnode.robot15__ui.robotui20__remove.robotDry-run clean: 5/5 under Robot 7.5 with SSHLibrary + robotframework-browser. Verified execution order is install -> configure -> route -> screenshots -> remove, and
--exclude ui(what CI does whenRUN_UI_TESTSis unset) correctly drops to 4.Two things that split would have broken, and did not:
Browseris imported inside the test body, not at suite level. The runner uses a different image without robotframework-browser when UI tests are off, so a suite-level import fails the whole run before the[ui]exclusion can skip the one test.${ADMIN_USER}/${ADMIN_PASSWORD}` moved with their only caller rather than staying behind in the install file..github/workflows/test-module.ymlDeliberately not the stock
test-module.yml@v1. It derives the image under test from the repo name (basename minus anns8-prefix, owner lowercased,mainforced tolatest), which for this repo gives:Two wrong answers at once:
ghcr.io/calnode/ns8-calnode, notghcr.io/calnode/calnode; the latter is the app image and carries no NS8 module manifest, soadd-modulewould fail:latestmodule tag. It stays pinned to releases, andmainpublishes:edge, exactly asdocker-publish.ymldoes for the appSo it calls
test-on-digitalocean-infra.yml@v1directly, which is the reusable workflowtest-module.ymldelegates to anyway, and supplies the image URL itself viaargs. DigitalOcean provisioning, NS8 install,create-cluster, the test runner, artifact upload and tmate debugging are all still theirs, unmodified. Full reasoning is in the workflow header so nobody later 'fixes' it back.Manual only, on purpose
It needs a
do_tokenrepo secret this repo does not have. Wiring theworkflow_runtrigger now would fail red on every publish, so it isworkflow_dispatchonly, with the trigger to uncomment documented alongside it.Also
.gitignore: robot writesoutput.xml,log.html,report.htmlintotests/outputs/, which was not ignored