tests: fix free /review URL test drift via a review_url output#123
Merged
Conversation
Add a `review_url` output to the breaking action (the free /review link it already emits as a notice) so tests can assert on it directly. Rewrite the two free-review URL tests to run the real action via `uses:` and check the output, instead of invoking the entrypoint on the runner under dash. They now run on the production platform (Alpine/busybox ash) with no shell-compat workarounds, and the strip test exercises real git-ref resolution (HEAD:specs/base.yaml), which had no CI coverage. Also align entrypoint drift: write_output now matches changelog/diff, add set -o pipefail (consistent with changelog/diff; safe now that every breaking test runs in-container), and drop an em-dash from a comment.
The review_url echo was between `breaking<<$delimiter` and its closing delimiter, so it got folded into the `breaking` output value (breaking the existing output-comparison tests) and never became its own output (so the URL tests saw it empty). Move it after the block closes.
Mirror the breaking change: add a review_url output (written after the multiline changelog block closes) and rewrite the two free-review URL tests to run the real action via uses: + assert on the output, dropping the bash/dash workaround. Also expands the base_sha comment to match breaking's (drift cleanup).
The entrypoints are #!/bin/sh. pipefail is not POSIX (the runner's dash rejects it), and it bought nothing here: every pipeline is either inside an if-condition or ends in the command whose failure already propagates. Standardize all four on `set -e` only, which also makes them runnable directly under the runner's dash.
reuvenharrison
changed the title
reuvenharrison
changed the title
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
1 participant
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.
The free-review URL tests had drifted: near-identical copies across the breaking/changelog/pr-comment workflows, a
bash-only workaround in changelog, and fragile entrypoint stubbing that ran under the runner'sdashinstead of the production shell. This makes them consistent and faithful.Tests (the drift)
review_urloutput to the breaking and changelog actions (the free/reviewlink they already emit as a::notice::) so a test can assert on it directly. Annotations and step logs can't be read by a later step; only declared outputs cross the step boundary.uses:and checksteps.*.outputs.review_url. They now run on the production platform (Alpine/busyboxash), with no stub /docker run/bashworkaround.base: HEAD:specs/base.yaml, adding the first CI coverage of real git-ref resolution (the recommended base form).pr-comment's tests are left as-is: it needs PR context, and 4 of them assert on exit codes / ARG_MAX behavior, so direct invocation underdashis the correct strict-POSIX gate. (ARG_MAX is kernel-level, so in-container adds nothing for those.)Entrypoint cleanup (minor, while here)
write_output:breakingmatched to thelocalversion used bychangelog/diff.base_shacomment:changelognow carries the full rationale instead of "see breaking" (and an em-dash removed).set -o pipefail: removed fromchangelog/diff. The entrypoints are#!/bin/sh;pipefailis non-POSIX and bought nothing here (every pipeline is in anifcondition or ends in the command whose failure already propagates). All four are nowset -eonly.Test plan
uses:tests unaffected by the entrypoint changes