ci: enforce deploy.sh and deploy.ps1 cross-platform tests on every branch - #29
Merged
Merged
Conversation
…anch deploy.sh and deploy.ps1 are load-bearing for every consumer of this library, but CI only covered deploy.ps1, only on main and PRs into main, and only when specific paths changed. deploy.sh was not in the path filter and had no workflow at all, so it could break without any signal. Triggers - Both workflows now run on every push and every pull request, with no branch or path filters. Add concurrency groups so in-flight runs are superseded. New deploy.sh workflow - shellcheck (--severity=warning) and bash -n over every tracked .sh file, as a fast parallel gate. - End-to-end tests/test-deploy.sh on ubuntu-latest and macos-latest, plus an explicit run under macOS /bin/bash 3.2 and a cwd-independence check. deploy.ps1 workflow - Add a fast parse + PSScriptAnalyzer compatibility gate (5.1 and 7.0). - Add macos-pwsh to the matrix alongside Windows PowerShell 5.1. - Fail if PSScriptAnalyzer is missing, since deploy.Tests.ps1 silently skips its compatibility test in that case, hiding 5.1 incompatibilities. Portability fixes found by the new macOS coverage - tests/test-deploy.sh used sha256sum and find -perm /111, both GNU-only. macOS has neither, so the suite could never have passed there. Replaced with helpers that fall back to shasum -a 256 and use -exec test -x for listing. - deploy.sh: remove a dead 'sequence' assignment so shellcheck is clean at warning severity, allowing CI to gate on it. Docs - Record the portability constraints, CI enforcement, and the rule against narrowing these triggers in the maintainer AGENTS.md. - Ignore testResults.xml, produced by Invoke-Pester -CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes correctly enforce always-on cross-platform CI for critical deploy scripts, with only a minor .gitignore anchoring nit identified.
Pull request overview
This PR strengthens CI enforcement for the repository’s two deploy entrypoints (deploy.sh and deploy.ps1) by ensuring they are continuously tested across supported platforms and shells, reflecting their “load-bearing” role for downstream consumers.
Changes:
- Added a new
deploy.shGitHub Actions workflow that runs ShellCheck +bash -n, and executes the end-to-end deploy test suite on Ubuntu and macOS (including explicit/bin/bash3.2 on macOS). - Updated the existing
deploy.ps1workflow to run on every push/PR (no branch/path filters), add a fast parse + compatibility gate, extend the test matrix to macOS PowerShell, and fail if PSScriptAnalyzer is unavailable. - Improved cross-platform portability in
tests/test-deploy.sh, removed dead assignment indeploy.sh, and documented deploy-script portability/CI non-negotiables inAGENTS.md(plus ignored Pester output).
File summaries
| File | Description |
|---|---|
| tests/test-deploy.sh | Adds portable checksum and executable-discovery helpers for macOS/BSD compatibility. |
| deploy.sh | Removes a dead variable assignment in interactive agent selection. |
| AGENTS.md | Documents portability constraints and CI trigger guarantees as maintainer non-negotiables/checklist items. |
| .gitignore | Ignores Pester CI output file testResults.xml. |
| .github/workflows/deploy-sh-tests.yml | Introduces always-on cross-platform CI for deploy.sh (lint + end-to-end). |
| .github/workflows/deploy-ps1-tests.yml | Expands deploy.ps1 CI to always-on, multi-OS + fast compatibility linting. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…t run pull_request already fires on the synchronize event, so a push to a branch with an open pull request was running the full matrix twice — including the ~4 minute Windows PowerShell 5.1 job. Coverage is unchanged: every pull request from any branch still runs both workflows on every commit, and main is still protected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new portable executable-file detection in tests/test-deploy.sh changes semantics from “any execute bit set” to “executable by current user,” reducing the idempotency test’s accuracy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+37
to
+38
| # -exec test -x is portable; GNU -perm /111 and BSD -perm +111 are not interchangeable. | ||
| list_executables() { find "$1" -type f -exec test -x {} \; -print | sort; } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deploy.shanddeploy.ps1are load-bearing — every consumer of this library runs one of them. CI did not reflect that:deploy.shhad no workflow at all, and was not even listed in thepaths:filter of the PowerShell workflow. It could break without any signal.mainand PRs intomain, and only when specific paths changed.Triggers
Both workflows now run on every push and every pull request, with no branch or path filters. These scripts must always work, so they are never allowed to go untested. Concurrency groups supersede in-flight runs so feedback tracks the latest push.
New
deploy.shworkflowshellcheck --severity=warningandbash -nover every tracked.shfile, discovered viagit ls-filesso new playbook scripts are covered automatically.tests/test-deploy.shonubuntu-latest(GNU userland) andmacos-latest(BSD userland), plus an explicit run under macOS/bin/bash3.2 —deploy.shdeclares#!/bin/bash, so that is what macOS consumers actually get — and a check thatdeploy.shworks from an arbitrary working directory.deploy.ps1workflowmacos-pwshto the matrix; Windows PowerShell 5.1 remains the oldest supported baseline.deploy.Tests.ps1marks its compatibility test-Skipwhen the module is absent, so an install failure would silently hide 5.1 incompatibilities.Portability bugs this immediately caught
Adding macOS coverage exposed two GNU-only constructs in
tests/test-deploy.shthat meant the suite could never have passed on macOS:sha256sum— does not exist on macOS (it shipsshasum).find -perm /111— GNU syntax; BSDfindrejects it outright.Both replaced with portable helpers (
shasum -a 256fallback,-exec test -x {} \; -print). Verified by forcing each branch: 58/58 pass viasha256sumand 58/58 viashasum, with byte-identical output.Also removed a dead
sequence=""assignment indeploy.shso the tree is clean at--severity=warning, which lets CI gate on it rather than only on hard errors.Docs
Recorded the portability constraints (no bash 4+ syntax or GNU-only utilities; no PowerShell 7-only syntax), the CI enforcement, and an explicit rule against narrowing these triggers in the maintainer
AGENTS.md— added to both Non-Negotiables and the Decision Checklist. IgnoredtestResults.xmlproduced byInvoke-Pester -CI.Validation
tests/test-deploy.sh58/58 (both checksum code paths).deploy.Tests.ps120/20, 0 skipped — the analyzer test now genuinely runs.shellcheck --severity=warningandbash -nclean across all 5 tracked shell scripts.mainbranch — the behaviour that previously did not exist.