-
Notifications
You must be signed in to change notification settings - Fork 370
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
cmd: Update each index instead of just default #588
Changes from 7 commits
d9f30f8
de38a18
6717fe8
a4c94da
0e31f9a
c674426
993758c
8a8195d
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 |
---|---|---|
|
@@ -49,6 +49,51 @@ func TestKrewUpdate(t *testing.T) { | |
} | ||
} | ||
|
||
func TestKrewUpdateMultipleIndexes(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
defer cleanup() | ||
|
||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex() | ||
// to enable new paths in environment.NewPaths() | ||
os.Setenv(constants.EnableMultiIndexSwitch, "1") | ||
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. Please add a comment like // to enable new paths in environment.NewPaths() |
||
defer os.Unsetenv(constants.EnableMultiIndexSwitch) | ||
|
||
paths := environment.NewPaths(test.Root()) | ||
test.Krew("index", "add", "foo", paths.IndexPath(constants.DefaultIndexName)).RunOrFail() | ||
if err := os.RemoveAll(paths.IndexPluginsPath("foo")); err != nil { | ||
t.Errorf("error removing plugins directory from index: %s", err) | ||
} | ||
test.Krew("update").RunOrFailOutput() | ||
out := string(test.Krew("search").RunOrFailOutput()) | ||
if !strings.Contains(out, "\nctx") { | ||
t.Error("expected plugin ctx in list of plugins") | ||
} | ||
if !strings.Contains(out, "foo/ctx") { | ||
t.Error("expected plugin foo/ctx in list of plugins") | ||
} | ||
} | ||
|
||
func TestKrewUpdateFailedIndex(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
defer cleanup() | ||
|
||
test = test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithIndex() | ||
os.Setenv(constants.EnableMultiIndexSwitch, "1") | ||
defer os.Unsetenv(constants.EnableMultiIndexSwitch) | ||
|
||
paths := environment.NewPaths(test.Root()) | ||
test.TempDir().InitEmptyGitRepo(paths.IndexPath("foo"), "invalid-git") | ||
out, err := test.Krew("update").Run() | ||
if err == nil { | ||
t.Error("expected update to fail") | ||
} | ||
if !strings.Contains(string(out), "failed to update the following indexes: foo") { | ||
t.Error("expected index update to fail for foo") | ||
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. Try doing this for debuggability: %q doesn’t contain msg=%q 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. What are the two strings in the example you gave? I'm guessing the output of 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. Out & “failed to ...foo” You can export latter into a variable. |
||
} | ||
} | ||
|
||
func TestKrewUpdateListsNewPlugins(t *testing.T) { | ||
skipShort(t) | ||
test, cleanup := NewTest(t) | ||
|
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.
I’ve realized this if (+counting) isnt necessary since errors.Wrap already handles nil err. Maybe remove?
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.
Sure I can do that. Do you think it will look weird if this ends with the
return errors.Wrapf(...)
?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.
I think it’s fine. I just realized the check is redundant.
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'll remove the check then.