-
Notifications
You must be signed in to change notification settings - Fork 371
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
multi-index: Add default index if none exists #595
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ package integrationtest | |
|
||
import ( | ||
"os" | ||
"regexp" | ||
"strings" | ||
"testing" | ||
|
||
|
@@ -171,3 +172,57 @@ func TestKrewIndexRemoveForce_nonExisting(t *testing.T) { | |
// --force returns success for non-existing indexes | ||
test.Krew("index", "remove", "--force", "non-existing").RunOrFail() | ||
} | ||
|
||
func TestKrewDefaultIndex_notAutomaticallyAdded(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1) | ||
defer cleanup() | ||
|
||
test.Krew("help").RunOrFail() | ||
out, err := test.Krew("search").Run() | ||
if err == nil { | ||
t.Fatalf("search must've failed without any indexes. output=%s", string(out)) | ||
} | ||
out = test.Krew("index", "list").RunOrFailOutput() | ||
if len(lines(out)) > 1 { | ||
t.Fatalf("expected no indexes; got output=%q", string(out)) | ||
} | ||
} | ||
|
||
func TestKrewDefaultIndex_AutoAddedOnInstall(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1) | ||
defer cleanup() | ||
|
||
test.Krew("install", validPlugin).RunOrFail() | ||
ensureIndexListHasDefaultIndex(t, string(test.Krew("index", "list").RunOrFailOutput())) | ||
} | ||
|
||
func TestKrewDefaultIndex_AutoAddedOnUpdate(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1) | ||
defer cleanup() | ||
|
||
test.Krew("update").RunOrFail() | ||
ensureIndexListHasDefaultIndex(t, string(test.Krew("index", "list").RunOrFailOutput())) | ||
} | ||
|
||
func TestKrewDefaultIndex_AutoAddedOnUpgrade(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1) | ||
defer cleanup() | ||
|
||
test.Krew("upgrade").RunOrFail() | ||
ensureIndexListHasDefaultIndex(t, string(test.Krew("index", "list").RunOrFailOutput())) | ||
} | ||
|
||
func ensureIndexListHasDefaultIndex(t *testing.T, output string) { | ||
t.Helper() | ||
if !regexp.MustCompile(`(?m)^default\b`).MatchString(output) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: isn't it better to put this at the package level so that the regular expression is only parsed once instead of for each time this function is called? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since it's only part of integration tests, I figured it doesn't matter a whole lot. |
||
t.Fatalf("index list did not return default index:\n%s", output) | ||
} | ||
} |
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.
Seems like you could just pass
*ITest
here as the param.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.
🤔 why? Only testing.T seems sufficient?
For now I prefer we do list exec outside this.
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.
Ok, I only mentioned since the
string(test.Krew("index", "list").RunOrFailOutput())
is being passed to each one. I thought it might be better to put that in theensureIndexListHasDefaultIndex
function rather than relying on the caller to pass the desired output.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.
yeah got it now. I think that's fine. this way we can actually see the error from RunOrFail at the correct line number.