test(magician): refactor test-terraform-vcr to use testing sandbox#17956
test(magician): refactor test-terraform-vcr to use testing sandbox#17956majedalfaraj wants to merge 2 commits into
Conversation
Migrates `test_terraform_vcr_test.go` to replace `mockRunner` with the sandbox testing framework. To support the use of the physical filesys, `test_terraform_vcr.go` was updated to dynamically resolve `diff_comment_id.txt`'s path using the new `WORKSPACE` env variable (defaulting to `/workspace`). This preserves behaviour in production, while allowing local testing in a temporary directory.
modular-magician
added
the
awaiting-approval
|
Googlers: For automatic test runs see go/terraform-auto-test-runs. @slevenick, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
modular-magician
removed
the
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 89076f4: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
| }, | ||
| } | ||
| sb := newSandbox(t) | ||
| os.Setenv("WORKSPACE", sb.Dir) |
There was a problem hiding this comment.
I don't love setting env vars in a test. Can we do this another way, like by passing the workspace into the runner/sandbox at some point?
There was a problem hiding this comment.
Good idea! I changed the functions and method signatures to include a workspace argument instead. Instead of adding new fields to the sandbox (which would impact the other active PRs), I updated this test's helper functions to include an extra workspace parameter. This allows the test to pass the sandbox's directory to the helpers for local testing, while the production code passes /workspace instead.
…erraform-vcr Update `test_terraform_vcr.go` to accept the workspace directory as a new function argument instead of creating and reading from the OS environment variables. Allows the tests to pass `sb.Dir` for local tests whilst passing `/workspace` for the default prod behaviour.
modular-magician
added
the
awaiting-approval
|
@slevenick This PR is approved and has been waiting for merge for 1 week. Is it ready to merge? Use the label |
3 participants
Migrates
test_terraform_vcr_test.goto replacemockRunnerwith the sandbox testing framework. To support the use of the physical filesys,test_terraform_vcr.gowas updated to dynamically resolvediff_comment_id.txt's path using the newWORKSPACEenv variable (defaulting to/workspace). This preserves behaviour in production, while allowing local testing in a temporary directory.