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

[HPR-1100] Add basic test for commonServer.ConfigSpec #176

Merged
merged 3 commits into from
Apr 25, 2023

Conversation

nywilken
Copy link
Contributor

@nywilken nywilken commented Apr 21, 2023

The Packer SDK relies on encoding/gob for serializing hcldec.ObjectSpec over RPC. The added test check that types implementing the HCL2Speccer interface can be encoded/decoded without issue.

Starting in go-cty v1.11.0 support for encoding/gob was removed, which causes the added tests to fail. This is expected and is meant to help catch potential regressions moving forward.

Test results running go-cty v1.10.0

--- PASS: TestCommonServer_ConfigSpec (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Datasource_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Provisioner_Component_Server (0.00s)
=== RUN   TestCommunicatorRPC

Test results running go-cty v1.13.1

~> go get github.com/zclconf/[email protected]
~>  go test ./rpc/... -v -run=TestCommonServer
=== RUN   TestCommonServer_ConfigSpec
=== RUN   TestCommonServer_ConfigSpec/Builder_Component_Server
    common_test.go:75: Call to ConfigSpec for Builder Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Datasource_Component_Server
    common_test.go:75: Call to ConfigSpec for Datasource Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Provisioner_Component_Server
    common_test.go:75: Call to ConfigSpec for Provisioner Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
--- FAIL: TestCommonServer_ConfigSpec (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Datasource_Component_Server (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Provisioner_Component_Server (0.00s)
FAIL
FAIL    github.com/hashicorp/packer-plugin-sdk/rpc      0.278s
FAIL

Using local fork of zclconf/go-ctyv1.12.1 with encoding/gob

~>  go test ./rpc/... -v -run=TestCommonServer
=== RUN   TestCommonServer_ConfigSpec
=== RUN   TestCommonServer_ConfigSpec/Builder_Component_Server
2023/04/21 14:59:37 [WARN] Shutting down mux conn in Server
2023/04/21 14:59:37 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Datasource_Component_Server
2023/04/21 14:59:37 [WARN] Shutting down mux conn in Server
2023/04/21 14:59:37 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Provisioner_Component_Server
2023/04/21 14:59:37 [WARN] Shutting down mux conn in Server
2023/04/21 14:59:37 [WARN] Client is closing mux
--- PASS: TestCommonServer_ConfigSpec (0.01s)
    --- PASS: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Datasource_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Provisioner_Component_Server (0.00s)
PASS
ok      github.com/hashicorp/packer-plugin-sdk/rpc      0.321s

Wilken Rivera added 2 commits April 21, 2023 11:43
The Packer SDK relies on encoding/gob for serializing hcldec.ObjectSpec
over RPC. The added test check that types implementing the HCL2Speccer
interface can be encoded/decoded without issue.

Starting in go-cty v1.11.0 support for encoding/gob was removed, which
causes the added tests to fail. This is expected and is meant to help
catch potential regressions moving forward.

Test results running go-cty v1.10.0
```
--- PASS: TestCommonServer_ConfigSpec (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Datasource_Component_Server (0.00s)
    --- PASS: TestCommonServer_ConfigSpec/Provisioner_Component_Server (0.00s)
=== RUN   TestCommunicatorRPC
```

Test results running go-cty v1.13.1
```
2023/04/21 11:28:05 [WARN] Client is closing mux
--- FAIL: TestCommonServer_ConfigSpec (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
panic: ConfigSpec failed: gob: type cty.Type has no exported fields [recovered]
        panic: ConfigSpec failed: gob: type cty.Type has no exported fields

goroutine 299 [running]:
testing.tRunner.func1.2({0x10524fb60, 0x14000486530})
        /usr/local/go/src/testing/testing.go:1389 +0x1c8
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1392 +0x384
panic({0x10524fb60, 0x14000486530})
        /usr/local/go/src/runtime/panic.go:838 +0x204
github.com/hashicorp/packer-plugin-sdk/rpc.(*commonClient).ConfigSpec(0x14000475f08)
        /Users/dev/Development/packer-plugin-sdk/rpc/common.go:48 +0x24c
github.com/hashicorp/packer-plugin-sdk/rpc.TestCommonServer_ConfigSpec.func1(0x140004e5860)
        /Users/dev/Development/packer-plugin-sdk/rpc/common_test.go:44 +0x10c
testing.tRunner(0x140004e5860, 0x14000483dd0)
        /usr/local/go/src/testing/testing.go:1439 +0x110
created by testing.(*T).Run
        /usr/local/go/src/testing/testing.go:1486 +0x300
FAIL    github.com/hashicorp/packer-plugin-sdk/rpc      0.796s
```
```
~> go get github.com/zclconf/[email protected]
~>  go test ./rpc/... -v -run=TestCommonServer
=== RUN   TestCommonServer_ConfigSpec
=== RUN   TestCommonServer_ConfigSpec/Builder_Component_Server
    common_test.go:75: Call to ConfigSpec for Builder Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Datasource_Component_Server
    common_test.go:75: Call to ConfigSpec for Datasource Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
=== RUN   TestCommonServer_ConfigSpec/Provisioner_Component_Server
    common_test.go:75: Call to ConfigSpec for Provisioner Component Server panicked: ConfigSpec failed: gob: type cty.Type has no exported fields
2023/04/21 14:31:27 [WARN] Shutting down mux conn in Server
2023/04/21 14:31:27 [WARN] Client is closing mux
--- FAIL: TestCommonServer_ConfigSpec (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Builder_Component_Server (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Datasource_Component_Server (0.00s)
    --- FAIL: TestCommonServer_ConfigSpec/Provisioner_Component_Server (0.00s)
FAIL
FAIL    github.com/hashicorp/packer-plugin-sdk/rpc      0.278s
FAIL
```
@nywilken nywilken requested a review from a team as a code owner April 21, 2023 18:52
@nywilken nywilken changed the title nywilken/gob encoding tests Add basic test for commonServer.ConfigSpec Apr 21, 2023

defer func() {
if r := recover(); r != nil {
t.Errorf("Call to ConfigSpec for %s panicked: %v", tc.name, r)
Copy link
Contributor Author

@nywilken nywilken Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to recover from the panic and fail the test so that we can add a help message to developers with instructions on how to resolve the go-cty encoding issue. It also allows for all component types to be tested without completely bailing.

@nywilken nywilken changed the title Add basic test for commonServer.ConfigSpec [HPR-1100] Add basic test for commonServer.ConfigSpec Apr 24, 2023
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just left one comment, but aside from that, LGTM!

packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
)

var b testing.B
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used anywhere, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL - good catch. That is a left over from my empty test file.

rpc/common_test.go Outdated Show resolved Hide resolved
@nywilken nywilken force-pushed the nywilken/gob-encoding-tests branch from 4e5a784 to 0c383dd Compare April 25, 2023 00:07
@nywilken nywilken merged commit b08070c into main Apr 25, 2023
@nywilken nywilken deleted the nywilken/gob-encoding-tests branch April 25, 2023 20:40
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.

2 participants