Closed
Conversation
reconcilerio@e07a96c#diff-ced16713406a9137f85f9e4b12a81ca15c3196b57eaa12ab71dfbb3cb82d7037R149-R157 broke all unit tests because validation would run in https://github.com/reconcilerio/runtime/blob/main/testing/reconciler.go#L226-L230 but the context wouldn’t contain a stashed config. Signed-off-by: Daniel Lohse <info@asapdesign.de>
da23f6e to
763a1ab
Compare
scothis
reviewed
Sep 27, 2025
Comment on lines
+217
to
+219
| c := expectConfig.Config() | ||
| ctx = reconcilers.StashConfig(ctx, c) | ||
| ctx = reconcilers.StashOriginalConfig(ctx, c) |
Member
There was a problem hiding this comment.
The config should not be stashed by the test harness because that's the ResourceReconciler's responsibility.
The root cause of the issue here is that the Validate method is called directly as part of the test. At runtime, Validate is called within the SetupWithManager method, and the context is configured there.
runtime/reconcilers/resource.go
Lines 179 to 184 in ec87995
I think the right approach here is to also setup the context when the entry point is the Validate method.
Created #654
Contributor
Author
There was a problem hiding this comment.
That makes sense. I'll close this PR.
Contributor
Author
|
Closing this, fixed in #654. |
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
2 participants
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.
e07a96c#diff-ced16713406a9137f85f9e4b12a81ca15c3196b57eaa12ab71dfbb3cb82d7037R149-R157 broke all unit tests because validation would run in https://github.com/reconcilerio/runtime/blob/main/testing/reconciler.go#L226-L230 and https://github.com/reconcilerio/runtime/blob/main/testing/subreconciler.go#L249-L253 but the context wouldn’t contain a stashed config.
Here are the test failures from our local suite:
Stashing the config and the original config (the latter only for consistency because it's done elsewhere in tandem as well) fixed our test suites (at least the panics).