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

make tests pass (or skip) on darwin/arm64 #304

Merged
merged 2 commits into from
May 18, 2022
Merged

make tests pass (or skip) on darwin/arm64 #304

merged 2 commits into from
May 18, 2022

Conversation

radeksimko
Copy link
Member

Why?
Prior to this patch, many tests would fail for the same reason - attempt to install older version of Terraform which isn't available for darwin/arm64.

For example:

--- FAIL: TestDestroyCmd (0.08s)
    terraform_test.go:164: caching exec "v:0.12.31" in dir "/var/folders/9w/pddq2x656j76vxvsbhcs4nwr0000gq/T/tfinstall1718612104/v-0.12.31"
panic: error installing terraform "v:0.12.31": no ZIP archive found for terraform <nil> darwin/arm64 [recovered]
	panic: error installing terraform "v:0.12.31": no ZIP archive found for terraform <nil> darwin/arm64

goroutine 99 [running]:
testing.tRunner.func1.2({0x102b2d280, 0x1400038cb80})
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/testing/testing.go:1389 +0x1c8
testing.tRunner.func1()
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/testing/testing.go:1392 +0x384
panic({0x102b2d280, 0x1400038cb80})
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/panic.go:838 +0x204
github.com/hashicorp/terraform-exec/tfexec/internal/testutil.(*TFCache).find(0x140002ca0c0, 0x14000003d40, {0x140004cc690, 0x9}, 0x14000077ea0)
	/Users/radeksimko/gopath/src/github.com/hashicorp/terraform-exec/tfexec/internal/testutil/tfcache_find.go:47 +0x374
github.com/hashicorp/terraform-exec/tfexec/internal/testutil.(*TFCache).Version(0x102b2d280?, 0x14000003d40, {0x1029fefa5, 0x7})
	/Users/radeksimko/gopath/src/github.com/hashicorp/terraform-exec/tfexec/internal/testutil/tfcache.go:61 +0x90
github.com/hashicorp/terraform-exec/tfexec.tfVersion(0x14000003d40?, {0x1029fefa5?, 0x14000067f48?})
	/Users/radeksimko/gopath/src/github.com/hashicorp/terraform-exec/tfexec/terraform_test.go:164 +0x88
github.com/hashicorp/terraform-exec/tfexec.TestDestroyCmd(0x14000003d40)
	/Users/radeksimko/gopath/src/github.com/hashicorp/terraform-exec/tfexec/destroy_test.go:13 +0x44
testing.tRunner(0x14000003d40, 0x102bf6820)
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/testing/testing.go:1439 +0x110
created by testing.(*T).Run
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/testing/testing.go:1486 +0x300
FAIL	github.com/hashicorp/terraform-exec/tfexec	6.097s

Alternatives

  • We could build Terraform as part of running relevant tests. That would likely add to the test time, increase the complexity of already complex matrix of versions throughout all packages and risk that a particular version may simply not work on darwin/arm64 as it was never properly tested.
  • We could update all tests to use v1.0.2+ which have darwin/arm64 builds, but there are still tests for older TF versions which may be useful to run.

Solution

I tried to strike the balance and bump TF version where it was possible/easy to do and skipped test where it wasn't.


I wish we could run any arm64 tests in CI, but AFAIK GitHub Actions don't offer that (yet).

Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Looks good to me, just some small non-blocking considerations 🚀

tfexec/internal/e2etest/show_test.go Outdated Show resolved Hide resolved
tfexec/init_test.go Show resolved Hide resolved
tfexec/terraform_test.go Outdated Show resolved Hide resolved
tfexec/workspace_show_test.go Outdated Show resolved Hide resolved
@radeksimko radeksimko merged commit fae0ba3 into main May 18, 2022
@radeksimko radeksimko deleted the t-darwin-arm64 branch May 18, 2022 15:12
rstandt pushed a commit to rstandt/terraform-exec that referenced this pull request Jun 21, 2022
* make tests pass on darwin/arm64

* address PR feedback (add extra v1 tests)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants