A setup can run a second time - #47
Merged
Merged
Conversation
Both halves of the metrics setup refused a repeat, and only one of them said so. registerSystemMetrics raised 'conflict': every class merges its declarations into the same node, and a leaf bound to a live object refused to merge with a second one even when the second one declared exactly the same thing. CanopyAccessor now recognises that case - same object, same selectors - as the one declaration made again rather than two wanting the same place. A real collision is still loud, and so is the one CanopyMergeTest has guarded since 2026-07-21: two accessors that declare nothing are not one declaration twice, they are none. registerZincMetrics raised on the domain, and before it did, the counter had already subscribed a second time - so the announcer handed every request to both and the counter silently counted double. install is now once, uninstall says so, and the registration re-declares through the merge above instead of failing. Canopy class>>useNewInstanceDuring: is the other half, for the callers rather than the registry. A test that registers domains of its own used to clean up with #reset, which is right in an image that exists for the test run. These test classes travel into the server images, though, and AGTestRunnerHandler runs any suite in the image it is asked for over /health - there #reset takes the tree the server is serving, #instance lazily builds an empty one, setupMetrics never runs again, and /metrics answers 200 with an empty body. A scrape cannot tell that from silence. Verified in image [a] before committing: a second registerSystemMetrics and a second registerZincMetrics both pass, the counter keeps exactly one subscription, useNewInstanceDuring: hands back the tree it was given, and two accessors on two distinct objects still conflict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
testUseNewInstanceDuringPutsThePreviousTreeBack has to put something into the ambient tree to have anything to get back, and it left it there - so testRegisterSystemMetricsTwiceLeavesTheSameTree found #somethingTheServerServes beside #pharo and failed on Pharo 13. The leak it left is the very thing it exists to prevent, which is worth saying out loud rather than quietly wrapping. Reset at both ends now: the leading one makes it independent of whatever ran before, the trailing one of what runs after. Both orderings and two runs in a row checked in image [a] this time by running the suite, not by evaluating the assertions one at a time - which is how this got through in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Both halves of the metrics setup refused a repeat, and only one of them said so.
registerSystemMetrics raised 'conflict': every class merges its declarations into the same node, and a leaf bound to a live object refused to merge with a second one even when the second one declared exactly the same thing. CanopyAccessor now recognises that case - same object, same selectors - as the one declaration made again rather than two wanting the same place. A real collision is still loud, and so is the one CanopyMergeTest has guarded since 2026-07-21: two accessors that declare nothing are not one declaration twice, they are none.
registerZincMetrics raised on the domain, and before it did, the counter had already subscribed a second time - so the announcer handed every request to both and the counter silently counted double. install is now once, uninstall says so, and the registration re-declares through the merge above instead of failing.
Canopy class>>useNewInstanceDuring: is the other half, for the callers rather than the registry. A test that registers domains of its own used to clean up with #reset, which is right in an image that exists for the test run. These test classes travel into the server images, though, and AGTestRunnerHandler runs any suite in the image it is asked for over /health - there #reset takes the tree the server is serving, #instance lazily builds an empty one, setupMetrics never runs again, and /metrics answers 200 with an empty body. A scrape cannot tell that from silence.
Verified in image [a] before committing: a second registerSystemMetrics and a second registerZincMetrics both pass, the counter keeps exactly one subscription, useNewInstanceDuring: hands back the tree it was given, and two accessors on two distinct objects still conflict.