ci: make the licensing matrix measure the license return - #278
Merged
Merged
Conversation
The return ran as `... || true` with its output discarded and its result never written into the cell, so the matrix measured activation only and reported 8/8 green while the return failed on every one of them. That is how #277 reached users: a run with six Unity versions warned "Failed to return the Personal license seat" on all six, on versions this matrix called healthy. A matrix that measures half a round trip is worse than no matrix, because it is what people point at to conclude there is no problem. The return now gets the same treatment as the activation - positive evidence from the log rather than an exit code, since Unity returns the licence and then exits non-zero - and a Return column beside Activate. `continue-on-error` stays so that one cell failing to return cannot abandon the other cells' returns. Three states, matching the activation probe's, plus one the activation has no equivalent for: "nothing held", which is a correct outcome rather than a failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The first version of this step restated the scripts' success strings, and the copy was already stale the first time it ran: the scripts had learned "Successfully returned the entitlement license" - the editor's wording for a Personal seat, and the only one that matters for the personal cells - so the matrix graded a verified successful return as a failure. Its own comment claimed it judged "from the same strings, rather than inventing a second opinion". Now it actually does: the pattern is read out of return_license.sh at run time, and a cell that cannot read it reports `error` rather than silently grading against nothing. That extraction is a text match against a variable name, so it would break silently if the variable were renamed. Two tests cover it - confirmed that a consistent rename fails exactly those two and nothing else. scripts/test-licensing-steps.sh: 86 -> 88 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every cell runs `activate` and `return-license` as separate commands, so they
get separate containers and separate machine identities, and a serial return is
refused with "Machine bindings don't match" every time. That says nothing about
whether the code works - a real build sources both in one container - so the
first run of this step graded all four serial cells as unreturnable.
Grading them red would park four permanently failing cells in the table, and a
table with cells everyone knows to ignore is the thing this workflow exists to
avoid. They now report `unmeasurable` ("separate container"), keyed on the
script's own permanent pattern, read from the script for the same reason
RETURNED is.
The fail/error discriminator was wrong too: it looked only for the three
command spellings, which the serial branch never prints, so a genuine return
failure was graded "probe never reached Unity". It now also accepts any
"Licensing::" line, which is the evidence that Unity's licensing module ran.
Measured on this branch: all four personal cells now grade `pass` - 2020.3.49f1,
2022.3.62f3, 6000.0.36f1 and 6000.6.0f1 activate AND return their Personal
seat. That round trip has never been green before, and until #277 it was never
even measured.
Still not covered: activate -> build -> return in ONE container, which is what a
real build does and where the serial return can actually succeed.
scripts/test-licensing-steps.sh: 88 -> 89 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closed
3 tasks
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 return ran as
... || truewith its output discarded and its result never written into the cell:So the matrix measured activation only, and reported 8/8 green while the return failed on every one of them.
That is how #277 reached users: a run with six Unity versions warned
Failed to return the Personal license seaton all six — on versions this matrix called healthy. A matrix that measures half a round trip is worse than no matrix, because it is what people point at to conclude there is no problem.The return now gets the same treatment the activation already gets — positive evidence read from the log rather than an exit code, since Unity returns the licence and then exits non-zero — and its own column:
Three states matching the activation probe's, plus one the activation has no equivalent for: nothing held, which is a correct outcome rather than a failure.
continue-on-errorstays, so one cell failing to return cannot abandon the other cells' returns.Merge after #277 — this is the check that confirms that fix actually releases the seat, rather than only stopping the false warning.
🤖 Generated with Claude Code