Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hcldec: A test case for attributes set to cty.DynamicVal with refinements #626

Merged
merged 1 commit into from Aug 30, 2023

Conversation

apparentlymart
Copy link
Member

@apparentlymart apparentlymart commented Aug 30, 2023

This test case is here to anticipate a possible bug that isn't actually buggy in the current implementation: if an attribute spec is given a non-dynamic type constraint and then refined based on that type constraint then the hcldec implementation must perform the type conversion first and only then attempt to add the refinements.

Another possible variation here would be for the attribute spec to have a dynamic type constraint (cty.DynamicPseudoType) and then try to refine its result. That case isn't tested here because that's always an implementation error in the calling application: RefineValueSpec must be used only in ways that are valid for the full range of types that the nested spec could produce, and there are no refinements that are valid for the full range of cty.DynamicPseudoType -- cty.DynamicVal is unrefineable. That situation can and will panic at runtime, alerting the application developer that they've used hcldec incorrectly. There is no way for end-user input to cause this panic if the calling application is written correctly.

This doesn't actually change the system behavior. It's a regression test to catch possible regressions under future maintenance. I added this test case after investigating a suspicion raised in #625, in which I found that the system currently behaves correctly in this case but that there was no test case verifying that to be true.

(The key difference between this and the bug in #625 is that typeexpr allows specifying type constraints as part of the end-user input -- the type constraint itself is decided dynamically at runtime -- whereas when using hcldec the spec should either be fixed at compile time as part of the application or generated from user input in some other place. In the latter case it's the calling application's responsibility to ensure that the dynamically-generated type constraint is valid before using it with hcldec.)

…ents

This test case is here to anticipate a _possible_ bug that isn't actually
buggy in the current implementation: if an attribute spec is given a
non-dynamic type constraint and then refined based on that type constraint
then the hcldec implementation must perform the type conversion first and
only then attempt to add the refinements.

Another possible variation here would be for the attribute spec to have
a dynamic type constraint (cty.DynamicPseudoType) and then try to refine
its result. That case isn't tested here because that's always an
implementation error in the calling application: RefineValueSpec must be
used only in ways that are valid for the full range of types that the
nested spec could produce, and there are no refinements that are valid
for the full range of cty.DynamicPseudoType. That situation can and will
panic at runtime, alerting the application developer that they've used
hcldec incorrectly. There is no way for end-user input to cause this panic
if the calling application is written correctly.

This doesn't actually change the system behavior. It's a regression test
to catch possible regressions under future maintenance.
@apparentlymart apparentlymart requested a review from a team August 30, 2023 15:52
@apparentlymart apparentlymart self-assigned this Aug 30, 2023
@apparentlymart apparentlymart merged commit 3617411 into main Aug 30, 2023
7 checks passed
@apparentlymart apparentlymart deleted the b-hcldec-refine-dynamic branch August 30, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants