breaking: review_url output; test free /review URL via it (drift cleanup)#123
Open
reuvenharrison wants to merge 3 commits into
Open
breaking: review_url output; test free /review URL via it (drift cleanup)#123reuvenharrison wants to merge 3 commits into
reuvenharrison wants to merge 3 commits into
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).
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.
First slice of the entrypoint drift / test-fidelity work, scoped to the breaking action so the approach can be validated before applying it to changelog and pr-comment.
What this does
review_urloutput on the breaking action (the free/reviewlink it already emits as a::notice::).uses: ./breakingand assert onsteps.brk.outputs.review_url— instead of invoking the entrypoint directly on the runner (dash). They now run on the production platform (Alpine/busyboxash), with no stub /docker run/bashworkaround.base: HEAD:specs/base.yaml, exercising real git-ref resolution (the action's recommended base form) which had no CI coverage.Drift alignment (breaking entrypoint)
write_outputnow matcheschangelog/diff(was a prefixed-var variant).set -o pipefailadded (consistent with changelog/diff; safe now that every breaking test runs in-container).Why a
review_urloutput instead of stubbingAnnotations (
::notice::) and step logs can't be read by a later step; only declared outputs cross the step boundary. Exposing the URL as an output lets the tests use the normaluses:+ output pattern (fully faithful) and is genuinely useful for real workflows.Follow-ups (same branch, after this is green)
pipefailstandardization still to be decided.Test plan
HEAD:resolves in CI)