-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Implement a function for workspace selection, or creation #164
Conversation
A new function `WorkspaceSelectOrNew` has been implemented and covered via three unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx!
modules/terraform/workspace.go
Outdated
return out, nil | ||
} | ||
|
||
if strings.Contains(out, name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems error prone. If you have a workspace called foobar
and you set name
to foo
, this will return true even though foo
doesn't actually exist. I think you need to split the output into individual lines and loop over each line looking for an exact match to name
. Note that the currently selected workspace may have an asterisk next to it, so it might require some extra cleaning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, I'll modify it.
|
||
// WorkspaceSelectOrNew runs terraform workspace with the given options and returns workspace name. | ||
// It tries to select a workspace with the given name, or it creates a new one if it doesn't exist. | ||
func WorkspaceSelectOrNew(t *testing.T, options *Options, name string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the return value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return value is the name of the current workspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add that to the docs!
modules/terraform/workspace.go
Outdated
_, err = RunTerraformCommandE(t, options, "workspace", "new", name) | ||
} | ||
if err != nil { | ||
return out, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return "", nil
@brikis98 Improved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thank you. One very minor change and this is good to merge!
} | ||
|
||
func nameMatchesWorkspace(name string, workspace string) bool { | ||
match, _ := regexp.MatchString(fmt.Sprintf("^\\*?\\s*%s$", name), workspace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment to explain what format you expect workspace
to have? It'll make it easier to understand the regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you! Merging now and will let tests run. When they pass, I'll create a new release.
Related to #163 (Terraform workspace support)
A new function
WorkspaceSelectOrNew
has been implemented and covered via three unit tests. The function takes a workspace name as an argument and: