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

Panic when handling required empty lists #1856

Open
t0yv0 opened this issue Apr 11, 2024 · 0 comments
Open

Panic when handling required empty lists #1856

t0yv0 opened this issue Apr 11, 2024 · 0 comments
Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec

Comments

@t0yv0
Copy link
Member

t0yv0 commented Apr 11, 2024

This is a bit of a degenerate case found in cross-tests under TestEmptyRequiredList in #1801 - specifically when there
is a Required list-nested block, TF disallows sending empty lists with an error, but Pulumi panics.

t0yv0 added a commit that referenced this issue Apr 12, 2024
Fixes #1790 by
building a rapid generator for schemas and associated values.

Large-ish problem 1: I do not have it figured out how to test unknown
values. TF literals as unknown values are forbidden and do not make
sense. We might need a helper resource so that testing unknown values
generates references to an output of the helper resource. This is logged
for future work.

Large-ish problem 2: iteration is pretty slow (x-proc). Normal n=100
rapid tests can take up to 10min. Could try batching so several
resources are tried in one shot say 100 resources.

Large-ish problem 3: I'm not sure if no-op Update and Create
implementations are acceptable. There is something to testing Computed
attributes where provider has to set values. Possibly Update also needs
to set values? Possibly not.

Small problems:

- [x] Using TF JSON syntax didn't handle null/empty correctly; that is
now discarded, using actual HCL syntax
- [x] TF representations are difficult to visualize in failing tests and
difficult to assert against
- [x] Lots of lost-in-translation papercuts possible between
representations (cty.Value, resource.PropertyValue, tftypes.Value)
- [x] this requires a change to providertest to abstract from testing.T
so we can pass rapid.T
- [x] it's very hard to disable annoying TF logging, using env vars for
now

We are starting to find bugs and discrepancies from this work:

- #1856 panic
corner-case
- #1852 need to
InternalValidate
- #1828

Future work:

- #1856 
- #1857 
- #1858 
- #1859 
- #1860 
- #1861 
- #1862 
- #1863 
- #1864 
- #1865 
- #1866 
- #1867
@iwahbe iwahbe added impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec labels Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

2 participants