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

TestConfigureSimpleValues test is flaky #2530

Open
VenelinMartinov opened this issue Oct 29, 2024 · 11 comments
Open

TestConfigureSimpleValues test is flaky #2530

VenelinMartinov opened this issue Oct 29, 2024 · 11 comments
Labels
impact/flaky-test A test that is unreliable kind/engineering Work that is not visible to an external user

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

The test seems to panic intermittently: https://github.com/pulumi/pulumi-terraform-bridge/actions/runs/11579563225/attempts/1?pr=2497

Example

.

Output of pulumi about

.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@VenelinMartinov VenelinMartinov added kind/engineering Work that is not visible to an external user p1 A bug severe enough to be the next item assigned to an engineer labels Oct 29, 2024
@VenelinMartinov VenelinMartinov changed the title ConfigureSimpleValues test is flaky TestConfigureSimpleValues test is flaky Oct 29, 2024
@VenelinMartinov
Copy link
Contributor Author

@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov added a commit that referenced this issue Oct 30, 2024
The test is flaky, so it should be skipped until we have time to dig
into it.

addresses #2530
@VenelinMartinov VenelinMartinov removed the p1 A bug severe enough to be the next item assigned to an engineer label Oct 30, 2024
@VenelinMartinov
Copy link
Contributor Author

Skipped the test, no longer a p1 but we should fix it

iwahbe added a commit that referenced this issue Oct 30, 2024
I'm unable to get the flake on my computer, even with `--count 1000`. The test wasn't
previously running in parallel, so it shouldn't be an interaction with other tests.

I've added better asserts so we can see which result is unexpectedly nil. I think our best
chance at fixing the test is to turn the test back on and then re-engage if/when it flakes
again.

Fixes #2530
iwahbe added a commit that referenced this issue Oct 31, 2024
I'm unable to get the flake on my computer, even with `--count 1000`.
The test wasn't previously running in parallel, so it shouldn't be an
interaction with other tests.

I've added better asserts so we can see which result is unexpectedly
nil. I think our best chance at fixing the test is to turn the test back
on and then re-engage if/when it flakes again.

Part of investigating
#2530
iwahbe added a commit that referenced this issue Nov 1, 2024
As part of diagnosing #2530, this
commit tightens `crosstests.Configure`. My assumption is that we will occasionally see
failures in the bridged provider here.
@iwahbe iwahbe self-assigned this Nov 1, 2024
iwahbe added a commit that referenced this issue Nov 1, 2024
As part of diagnosing #2530, this
commit tightens `crosstests.Configure`. My assumption is that we will occasionally see
failures in the bridged provider here.
iwahbe added a commit that referenced this issue Nov 1, 2024
As part of diagnosing #2530, this
commit tightens `crosstests.Configure`. My assumption is that we will occasionally see
failures in the bridged provider here.
iwahbe added a commit that referenced this issue Nov 1, 2024
As part of diagnosing #2530, this
commit fixes some assertions in `crosstests.Configure`. My assumption is that we will
occasionally see failures in the bridged provider here.
iwahbe added a commit that referenced this issue Nov 1, 2024
As part of diagnosing
#2530, this
commit tightens `crosstests.Configure`. My assumption is that we will
occasionally see failures in the bridged provider here.
@VenelinMartinov
Copy link
Contributor Author

@guineveresaenger
Copy link
Contributor

The occasions I saw were hitting on ubuntu default, but that might just be chance

@VenelinMartinov VenelinMartinov added p1 A bug severe enough to be the next item assigned to an engineer and removed p1 A bug severe enough to be the next item assigned to an engineer labels Nov 4, 2024
iwahbe added a commit that referenced this issue Nov 5, 2024
This PR tests the theory that provider creation or configuration may be skipped if they
are depended on by no resources.

Part of #2530
iwahbe added a commit that referenced this issue Nov 5, 2024
This PR tests the theory that provider creation or configuration may be skipped if they
are depended on by no resources.

Part of #2530
iwahbe added a commit that referenced this issue Nov 5, 2024
This PR tests the theory that provider creation or configuration may be skipped if they
are depended on by no resources.

Part of #2530
@VenelinMartinov
Copy link
Contributor Author

@iwahbe
Copy link
Member

iwahbe commented Nov 20, 2024

Hit this again: https://github.com/pulumi/pulumi-terraform-bridge/actions/runs/11935083949/job/33265640151?pr=2639

The error trace is

=== NAME  TestConfigure/set-attribute/empty
    configure.go:210: 
        	Error Trace:	/home/runner/work/pulumi-terraform-bridge/pulumi-terraform-bridge/pkg/pf/tests/internal/cross-tests/configure.go:210
        	            				/home/runner/work/pulumi-terraform-bridge/pulumi-terraform-bridge/pkg/pf/tests/internal/cross-tests/configure.go:59

This is the PF configure call. The rest of the issue has been about the SDKv2 Configure call.

@iwahbe iwahbe removed their assignment Nov 21, 2024
@iwahbe iwahbe added the impact/flaky-test A test that is unreliable label Nov 21, 2024
@VenelinMartinov
Copy link
Contributor Author

@iwahbe
Copy link
Member

iwahbe commented Nov 22, 2024

Is it the error that pulumi/pulumi#17825 is supposed to fix?

@VenelinMartinov
Copy link
Contributor Author

No, doesn't look like it

    configure.go:210: 
        	Error Trace:	/home/runner/work/pulumi-terraform-bridge/pulumi-terraform-bridge/pkg/pf/tests/internal/cross-tests/configure.go:210
        	            				/home/runner/work/pulumi-terraform-bridge/pulumi-terraform-bridge/pkg/pf/tests/internal/cross-tests/configure.go:59
        	Error:      	Not equal: 
        	            	expected: tfsdk.Config{Raw:tftypes.Value{typ:tftypes.Object{AttributeTypes:map[string]tftypes.Type{"k":tftypes.Set{ElementType:tftypes.primitive{name:"String", _:[]struct {}(nil)}, _:[]struct {}(nil)}}, OptionalAttributes:map[string]struct {}(nil), _:[]struct {}(nil)}, value:map[string]tftypes.Value{"k":tftypes.Value{typ:tftypes.Set{ElementType:tftypes.primitive{name:"String", _:[]struct {}(nil)}, _:[]struct {}(nil)}, value:interface {}(nil)}}}, Schema:schema.Schema{Attributes:map[string]schema.Attribute{"k":schema.SetAttribute{ElementType:basetypes.StringType{}, CustomType:basetypes.SetTypable(nil), Required:false, Optional:true, Sensitive:false, Description:"", MarkdownDescription:"", DeprecationMessage:"", Validators:[]validator.Set(nil)}}, Blocks:map[string]schema.Block(nil), Description:"", MarkdownDescription:"", DeprecationMessage:""}}
        	            	actual  : tfsdk.Config{Raw:tftypes.Value{typ:tftypes.Type(nil), value:interface {}(nil)}, Schema:fwschema.Schema(nil)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -2,48 +2,6 @@
        	            	  Raw: (tftypes.Value) {
        	            	-  typ: (tftypes.Object) {
        	            	-   AttributeTypes: (map[string]tftypes.Type) (len=1) {
        	            	-    (string) (len=1) "k": (tftypes.Set) {
        	            	-     ElementType: (tftypes.primitive) {
        	            	-      name: (string) (len=6) "String",
        	            	-      _: ([]struct {}) <nil>
        	            	-     },
        	            	-     _: ([]struct {}) <nil>
        	            	-    }
        	            	-   },
        	            	-   OptionalAttributes: (map[string]struct {}) <nil>,
        	            	-   _: ([]struct {}) <nil>
        	            	-  },
        	            	-  value: (map[string]tftypes.Value) (len=1) {
        	            	-   (string) (len=1) "k": (tftypes.Value) {
        	            	-    typ: (tftypes.Set) {
        	            	-     ElementType: (tftypes.primitive) {
        	            	-      name: (string) (len=6) "String",
        	            	-      _: ([]struct {}) <nil>
        	            	-     },
        	            	-     _: ([]struct {}) <nil>
        	            	-    },
        	            	-    value: (interface {}) <nil>
        	            	-   }
        	            	-  }
        	            	+  typ: (tftypes.Type) <nil>,
        	            	+  value: (interface {}) <nil>
        	            	  },
        	            	- Schema: (schema.Schema) {
        	            	-  Attributes: (map[string]schema.Attribute) (len=1) {
        	            	-   (string) (len=1) "k": (schema.SetAttribute) {
        	            	-    ElementType: (basetypes.StringType) {
        	            	-    },
        	            	-    CustomType: (basetypes.SetTypable) <nil>,
        	            	-    Required: (bool) false,
        	            	-    Optional: (bool) true,
        	            	-    Sensitive: (bool) false,
        	            	-    Description: (string) "",
        	            	-    MarkdownDescription: (string) "",
        	            	-    DeprecationMessage: (string) "",
        	            	-    Validators: ([]validator.Set) <nil>
        	            	-   }
        	            	-  },
        	            	-  Blocks: (map[string]schema.Block) <nil>,
        	            	-  Description: (string) "",
        	            	-  MarkdownDescription: (string) "",
        	            	-  DeprecationMessage: (string) ""
        	            	- }
        	            	+ Schema: (fwschema.Schema) <nil>
        	            	 }
        	Test:       	TestConfigure/set-attribute/missing

@iwahbe
Copy link
Member

iwahbe commented Nov 22, 2024

#2661 implements the same strategy as helped reduce TestConfigureSimpleValues flakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/flaky-test A test that is unreliable kind/engineering Work that is not visible to an external user
Projects
None yet
Development

No branches or pull requests

3 participants