DO NOT MERGE: negative test of CI enforcement - #30
Closed
ldastey-dev wants to merge 3 commits into
Closed
ldastey-dev wants to merge 3 commits into
ldastey-dev wants to merge 3 commits into
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>
…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
It introduces confirmed baseline-breaking incompatibilities (BSD find vs -perm /111, and PowerShell 5.1 parse failure) and also deviates from the repo’s documented actions/checkout@v4 convention.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CI enforcement for deploy script portability/compatibility (macOS bash 3.2 + BSD userland for deploy.sh, and Windows PowerShell 5.1 baseline for deploy.ps1) and updates maintainer guidance accordingly. As written, the PR also includes deliberate incompatibilities that will break the supported baselines (consistent with the PR description’s “negative test” intent).
Changes:
- Introduces GitHub Actions workflows to lint and run end-to-end tests for
deploy.sh(Ubuntu + macOS) anddeploy.ps1(Windows PowerShell 5.1 + pwsh across OSes), with added concurrency controls. - Updates
tests/test-deploy.shto use helper functions for tree checksums and executable discovery. - Documents portability/compatibility requirements in
AGENTS.mdand ignores Pester CI output.
File summaries
| File | Description |
|---|---|
| tests/test-deploy.sh | Adds checksum/executable helper functions and uses them in the idempotency test case. |
| deploy.sh | Minor change in interactive agent selection input handling (removes unused variable initialisation). |
| deploy.ps1 | Adds a variable assignment using PowerShell 7-only ternary syntax (currently breaks 5.1). |
| AGENTS.md | Expands explicit non-negotiables around bash 3.2/BSD portability and PS 5.1 compatibility + CI enforcement. |
| .gitignore | Ignores Pester CI output file testResults.xml. |
| .github/workflows/deploy-sh-tests.yml | New workflow to lint and run deploy.sh tests on Ubuntu + macOS (including /bin/bash). |
| .github/workflows/deploy-ps1-tests.yml | Enhances workflow to remove path filters, add lint gate, expand OS coverage, and harden analyzer availability. |
Review details
Suppressed comments (2)
.github/workflows/deploy-sh-tests.yml:76
- Repo guidance and examples use
actions/checkout@v4(seestandards/ci-cd.md:140). Pinning toactions/checkout@v7deviates from that convention and may reference a non-existent major version.
- uses: actions/checkout@v7
.github/workflows/deploy-ps1-tests.yml:93
- Repo guidance and examples use
actions/checkout@v4(seestandards/ci-cd.md:140). Pinning toactions/checkout@v7deviates from that convention and may reference a non-existent major version.
- uses: actions/checkout@v7
- Files reviewed: 6/7 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| name: parse and compatibility analysis | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v7 |
| name: shellcheck and syntax | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v7 |
Comment on lines
+550
to
551
| $script:NegativeTest = $true ? 'yes' : 'no' | ||
| $ScriptDir = $PSScriptRoot |
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 -perm /111 | 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.
Temporary draft PR verifying that the new workflows actually fail on breakage. Contains a deliberate GNU-only
find -perm /111(must fail macos-bash) and PowerShell 7-only ternary syntax (must fail the 5.1 compatibility gate). Will be closed and deleted.