2017-03-26 02:23:24 -04:00
|
|
|
package stack
|
|
|
|
|
|
|
|
import (
|
|
|
|
"errors"
|
2022-02-25 08:31:31 -05:00
|
|
|
"io"
|
2017-03-26 02:23:24 -04:00
|
|
|
"strings"
|
|
|
|
"testing"
|
|
|
|
|
2017-08-21 16:30:09 -04:00
|
|
|
"github.com/docker/cli/internal/test"
|
2020-02-22 12:12:14 -05:00
|
|
|
"gotest.tools/v3/assert"
|
|
|
|
is "gotest.tools/v3/assert/cmp"
|
2017-03-26 02:23:24 -04:00
|
|
|
)
|
|
|
|
|
2017-07-12 11:37:35 -04:00
|
|
|
func fakeClientForRemoveStackTest(version string) *fakeClient {
|
2017-03-26 02:23:24 -04:00
|
|
|
allServices := []string{
|
|
|
|
objectName("foo", "service1"),
|
|
|
|
objectName("foo", "service2"),
|
|
|
|
objectName("bar", "service1"),
|
|
|
|
objectName("bar", "service2"),
|
|
|
|
}
|
|
|
|
allNetworks := []string{
|
|
|
|
objectName("foo", "network1"),
|
|
|
|
objectName("bar", "network1"),
|
|
|
|
}
|
|
|
|
allSecrets := []string{
|
|
|
|
objectName("foo", "secret1"),
|
|
|
|
objectName("foo", "secret2"),
|
|
|
|
objectName("bar", "secret1"),
|
|
|
|
}
|
2017-05-26 20:30:33 -04:00
|
|
|
allConfigs := []string{
|
|
|
|
objectName("foo", "config1"),
|
|
|
|
objectName("foo", "config2"),
|
|
|
|
objectName("bar", "config1"),
|
|
|
|
}
|
2017-07-12 11:37:35 -04:00
|
|
|
return &fakeClient{
|
|
|
|
version: version,
|
2017-03-26 02:23:24 -04:00
|
|
|
services: allServices,
|
|
|
|
networks: allNetworks,
|
|
|
|
secrets: allSecrets,
|
2017-05-26 20:30:33 -04:00
|
|
|
configs: allConfigs,
|
2017-03-26 02:23:24 -04:00
|
|
|
}
|
2017-07-12 11:37:35 -04:00
|
|
|
}
|
|
|
|
|
2018-06-26 08:07:26 -04:00
|
|
|
func TestRemoveWithEmptyName(t *testing.T) {
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(test.NewFakeCli(&fakeClient{}))
|
2018-06-26 08:07:26 -04:00
|
|
|
cmd.SetArgs([]string{"good", "' '", "alsogood"})
|
2022-02-25 08:31:31 -05:00
|
|
|
cmd.SetOut(io.Discard)
|
test spring-cleaning
This makes a quick pass through our tests;
Discard output/err
----------------------------------------------
Many tests were testing for error-conditions, but didn't discard output.
This produced a lot of noise when running the tests, and made it hard
to discover if there were actual failures, or if the output was expected.
For example:
=== RUN TestConfigCreateErrors
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: error creating config
--- PASS: TestConfigCreateErrors (0.00s)
And after discarding output:
=== RUN TestConfigCreateErrors
--- PASS: TestConfigCreateErrors (0.00s)
Use sub-tests where possible
----------------------------------------------
Some tests were already set-up to use test-tables, and even had a usable
name (or in some cases "error" to check for). Change them to actual sub-
tests. Same test as above, but now with sub-tests and output discarded:
=== RUN TestConfigCreateErrors
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments#01
=== RUN TestConfigCreateErrors/error_creating_config
--- PASS: TestConfigCreateErrors (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments#01 (0.00s)
--- PASS: TestConfigCreateErrors/error_creating_config (0.00s)
PASS
It's not perfect in all cases (in the above, there's duplicate "expected"
errors, but Go conveniently adds "#01" for the duplicate). There's probably
also various tests I missed that could still use the same changes applied;
we can improve these in follow-ups.
Set cmd.Args to prevent test-failures
----------------------------------------------
When running tests from my IDE, it compiles the tests before running,
then executes the compiled binary to run the tests. Cobra doesn't like
that, because in that situation `os.Args` is taken as argument for the
command that's executed. The command that's tested now sees the test-
flags as arguments (`-test.v -test.run ..`), which causes various tests
to fail ("Command XYZ does not accept arguments").
# compile the tests:
go test -c -o foo.test
# execute the test:
./foo.test -test.v -test.run TestFoo
=== RUN TestFoo
Error: "foo" accepts no arguments.
The Cobra maintainers ran into the same situation, and for their own
use have added a special case to ignore `os.Args` in these cases;
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L1078-L1083
args := c.args
// Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155
if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" {
args = os.Args[1:]
}
Unfortunately, that exception is too specific (only checks for `cobra.test`),
so doesn't automatically fix the issue for other test-binaries. They did
provide a `cmd.SetArgs()` utility for this purpose
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L276-L280
// SetArgs sets arguments for the command. It is set to os.Args[1:] by default, if desired, can be overridden
// particularly useful when testing.
func (c *Command) SetArgs(a []string) {
c.args = a
}
And the fix is to explicitly set the command's args to an empty slice to
prevent Cobra from falling back to using `os.Args[1:]` as arguments.
cmd := newSomeThingCommand()
cmd.SetArgs([]string{})
Some tests already take this issue into account, and I updated some tests
for this, but there's likely many other ones that can use the same treatment.
Perhaps the Cobra maintainers would accept a contribution to make their
condition less specific and to look for binaries ending with a `.test`
suffix (which is what compiled binaries usually are named as).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 19:29:04 -04:00
|
|
|
cmd.SetErr(io.Discard)
|
2018-06-26 08:07:26 -04:00
|
|
|
|
|
|
|
assert.ErrorContains(t, cmd.Execute(), `invalid stack name: "' '"`)
|
|
|
|
}
|
|
|
|
|
2017-07-12 11:37:35 -04:00
|
|
|
func TestRemoveStackVersion124DoesNotRemoveConfigsOrSecrets(t *testing.T) {
|
|
|
|
client := fakeClientForRemoveStackTest("1.24")
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(test.NewFakeCli(client))
|
2017-03-26 02:23:24 -04:00
|
|
|
cmd.SetArgs([]string{"foo", "bar"})
|
|
|
|
|
2018-03-06 15:13:00 -05:00
|
|
|
assert.NilError(t, cmd.Execute())
|
2018-03-05 18:53:52 -05:00
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.services), client.removedServices))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.networks), client.removedNetworks))
|
|
|
|
assert.Check(t, is.Len(client.removedSecrets, 0))
|
|
|
|
assert.Check(t, is.Len(client.removedConfigs, 0))
|
2017-07-12 11:37:35 -04:00
|
|
|
}
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
|
2017-07-12 11:37:35 -04:00
|
|
|
func TestRemoveStackVersion125DoesNotRemoveConfigs(t *testing.T) {
|
|
|
|
client := fakeClientForRemoveStackTest("1.25")
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(test.NewFakeCli(client))
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
cmd.SetArgs([]string{"foo", "bar"})
|
|
|
|
|
2018-03-06 15:13:00 -05:00
|
|
|
assert.NilError(t, cmd.Execute())
|
2018-03-05 18:53:52 -05:00
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.services), client.removedServices))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.networks), client.removedNetworks))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.secrets), client.removedSecrets))
|
|
|
|
assert.Check(t, is.Len(client.removedConfigs, 0))
|
2017-07-12 11:37:35 -04:00
|
|
|
}
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
|
2017-07-12 11:37:35 -04:00
|
|
|
func TestRemoveStackVersion130RemovesEverything(t *testing.T) {
|
|
|
|
client := fakeClientForRemoveStackTest("1.30")
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(test.NewFakeCli(client))
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
cmd.SetArgs([]string{"foo", "bar"})
|
|
|
|
|
2018-03-06 15:13:00 -05:00
|
|
|
assert.NilError(t, cmd.Execute())
|
2018-03-05 18:53:52 -05:00
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.services), client.removedServices))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.networks), client.removedNetworks))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.secrets), client.removedSecrets))
|
|
|
|
assert.Check(t, is.DeepEqual(buildObjectIDs(client.configs), client.removedConfigs))
|
2017-03-26 02:23:24 -04:00
|
|
|
}
|
|
|
|
|
2017-07-05 13:32:54 -04:00
|
|
|
func TestRemoveStackSkipEmpty(t *testing.T) {
|
2017-03-26 02:23:24 -04:00
|
|
|
allServices := []string{objectName("bar", "service1"), objectName("bar", "service2")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allServiceIDs := buildObjectIDs(allServices)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
|
|
|
allNetworks := []string{objectName("bar", "network1")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allNetworkIDs := buildObjectIDs(allNetworks)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
|
|
|
allSecrets := []string{objectName("bar", "secret1")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allSecretIDs := buildObjectIDs(allSecrets)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
2017-05-26 20:30:33 -04:00
|
|
|
allConfigs := []string{objectName("bar", "config1")}
|
|
|
|
allConfigIDs := buildObjectIDs(allConfigs)
|
|
|
|
|
2017-07-05 13:32:54 -04:00
|
|
|
fakeClient := &fakeClient{
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
version: "1.30",
|
2017-03-26 02:23:24 -04:00
|
|
|
services: allServices,
|
|
|
|
networks: allNetworks,
|
|
|
|
secrets: allSecrets,
|
2017-05-26 20:30:33 -04:00
|
|
|
configs: allConfigs,
|
2017-03-26 02:23:24 -04:00
|
|
|
}
|
2017-07-05 14:19:52 -04:00
|
|
|
fakeCli := test.NewFakeCli(fakeClient)
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(fakeCli)
|
2017-03-26 02:23:24 -04:00
|
|
|
cmd.SetArgs([]string{"foo", "bar"})
|
|
|
|
|
2018-03-06 15:13:00 -05:00
|
|
|
assert.NilError(t, cmd.Execute())
|
2022-09-29 11:21:51 -04:00
|
|
|
expectedList := []string{
|
|
|
|
"Removing service bar_service1",
|
2017-08-31 22:42:34 -04:00
|
|
|
"Removing service bar_service2",
|
|
|
|
"Removing secret bar_secret1",
|
|
|
|
"Removing config bar_config1",
|
|
|
|
"Removing network bar_network1\n",
|
|
|
|
}
|
2018-03-05 18:53:52 -05:00
|
|
|
assert.Check(t, is.Equal(strings.Join(expectedList, "\n"), fakeCli.OutBuffer().String()))
|
|
|
|
assert.Check(t, is.Contains(fakeCli.ErrBuffer().String(), "Nothing found in stack: foo\n"))
|
|
|
|
assert.Check(t, is.DeepEqual(allServiceIDs, fakeClient.removedServices))
|
|
|
|
assert.Check(t, is.DeepEqual(allNetworkIDs, fakeClient.removedNetworks))
|
|
|
|
assert.Check(t, is.DeepEqual(allSecretIDs, fakeClient.removedSecrets))
|
|
|
|
assert.Check(t, is.DeepEqual(allConfigIDs, fakeClient.removedConfigs))
|
2017-03-26 02:23:24 -04:00
|
|
|
}
|
|
|
|
|
2017-06-20 14:00:01 -04:00
|
|
|
func TestRemoveContinueAfterError(t *testing.T) {
|
2017-03-26 02:23:24 -04:00
|
|
|
allServices := []string{objectName("foo", "service1"), objectName("bar", "service1")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allServiceIDs := buildObjectIDs(allServices)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
|
|
|
allNetworks := []string{objectName("foo", "network1"), objectName("bar", "network1")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allNetworkIDs := buildObjectIDs(allNetworks)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
|
|
|
allSecrets := []string{objectName("foo", "secret1"), objectName("bar", "secret1")}
|
Remove pkg/testutil/assert in favor of testify
I noticed that we're using a homegrown package for assertions. The
functions are extremely similar to testify, but with enough slight
differences to be confusing (for example, Equal takes its arguments in a
different order). We already vendor testify, and it's used in a few
places by tests.
I also found some problems with pkg/testutil/assert. For example, the
NotNil function seems to be broken. It checks the argument against
"nil", which only works for an interface. If you pass in a nil map or
slice, the equality check will fail.
In the interest of avoiding NIH, I'm proposing replacing
pkg/testutil/assert with testify. The test code looks almost the same,
but we avoid the confusion of having two similar but slightly different
assertion packages, and having to maintain our own package instead of
using a commonly-used one.
In the process, I found a few places where the tests should halt if an
assertion fails, so I've made those cases (that I noticed) use "require"
instead of "assert", and I've vendored the "require" package from
testify alongside the already-present "assert" package.
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
2017-04-13 18:45:37 -04:00
|
|
|
allSecretIDs := buildObjectIDs(allSecrets)
|
2017-03-26 02:23:24 -04:00
|
|
|
|
2017-05-26 20:30:33 -04:00
|
|
|
allConfigs := []string{objectName("foo", "config1"), objectName("bar", "config1")}
|
|
|
|
allConfigIDs := buildObjectIDs(allConfigs)
|
|
|
|
|
2017-03-26 02:23:24 -04:00
|
|
|
removedServices := []string{}
|
|
|
|
cli := &fakeClient{
|
Don't attempt to remove unsupported resources on older daemon
When running `docker stack rm <some stack>` against an older daemon,
a warning was printed for "configs" being ignored;
WARNING: ignoring "configs" (requires API version 1.30, but the Docker daemon API version is 1.26)
Given that an old daemon cannot _have_ configs, there should not be
a need to warn, or _attempt_ to remove these resources.
This patch removes the warning, and skips fetching (and removing)
configs.
A check if _secrets_ are supported by the daemon is also added,
given that this would result in an error when attempted against
an older (pre 1.13) daemon.
There is one situation where this could lead to secrets or
configs being left behind; if the client is connecting to a
daemon that _does_ support secrets, configs, but the API version
is overridden using `DOCKER_API_VERSION`, no warning is printed,
and secrets and configs are not attempted to be removed.
Given that `DOCKER_API_VERSION` is regarded a feature for
debugging / "power users", it should be ok to ignore this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2017-06-30 20:00:16 -04:00
|
|
|
version: "1.30",
|
2017-03-26 02:23:24 -04:00
|
|
|
services: allServices,
|
|
|
|
networks: allNetworks,
|
|
|
|
secrets: allSecrets,
|
2017-05-26 20:30:33 -04:00
|
|
|
configs: allConfigs,
|
2017-03-26 02:23:24 -04:00
|
|
|
|
|
|
|
serviceRemoveFunc: func(serviceID string) error {
|
|
|
|
removedServices = append(removedServices, serviceID)
|
|
|
|
|
|
|
|
if strings.Contains(serviceID, "foo") {
|
|
|
|
return errors.New("")
|
|
|
|
}
|
|
|
|
return nil
|
|
|
|
},
|
|
|
|
}
|
2022-02-22 07:46:35 -05:00
|
|
|
cmd := newRemoveCommand(test.NewFakeCli(cli))
|
2022-02-25 08:31:31 -05:00
|
|
|
cmd.SetOut(io.Discard)
|
test spring-cleaning
This makes a quick pass through our tests;
Discard output/err
----------------------------------------------
Many tests were testing for error-conditions, but didn't discard output.
This produced a lot of noise when running the tests, and made it hard
to discover if there were actual failures, or if the output was expected.
For example:
=== RUN TestConfigCreateErrors
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: "create" requires exactly 2 arguments.
See 'create --help'.
Usage: create [OPTIONS] CONFIG file|- [flags]
Create a config from a file or STDIN
Error: error creating config
--- PASS: TestConfigCreateErrors (0.00s)
And after discarding output:
=== RUN TestConfigCreateErrors
--- PASS: TestConfigCreateErrors (0.00s)
Use sub-tests where possible
----------------------------------------------
Some tests were already set-up to use test-tables, and even had a usable
name (or in some cases "error" to check for). Change them to actual sub-
tests. Same test as above, but now with sub-tests and output discarded:
=== RUN TestConfigCreateErrors
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments
=== RUN TestConfigCreateErrors/requires_exactly_2_arguments#01
=== RUN TestConfigCreateErrors/error_creating_config
--- PASS: TestConfigCreateErrors (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments (0.00s)
--- PASS: TestConfigCreateErrors/requires_exactly_2_arguments#01 (0.00s)
--- PASS: TestConfigCreateErrors/error_creating_config (0.00s)
PASS
It's not perfect in all cases (in the above, there's duplicate "expected"
errors, but Go conveniently adds "#01" for the duplicate). There's probably
also various tests I missed that could still use the same changes applied;
we can improve these in follow-ups.
Set cmd.Args to prevent test-failures
----------------------------------------------
When running tests from my IDE, it compiles the tests before running,
then executes the compiled binary to run the tests. Cobra doesn't like
that, because in that situation `os.Args` is taken as argument for the
command that's executed. The command that's tested now sees the test-
flags as arguments (`-test.v -test.run ..`), which causes various tests
to fail ("Command XYZ does not accept arguments").
# compile the tests:
go test -c -o foo.test
# execute the test:
./foo.test -test.v -test.run TestFoo
=== RUN TestFoo
Error: "foo" accepts no arguments.
The Cobra maintainers ran into the same situation, and for their own
use have added a special case to ignore `os.Args` in these cases;
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L1078-L1083
args := c.args
// Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155
if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" {
args = os.Args[1:]
}
Unfortunately, that exception is too specific (only checks for `cobra.test`),
so doesn't automatically fix the issue for other test-binaries. They did
provide a `cmd.SetArgs()` utility for this purpose
https://github.com/spf13/cobra/blob/v1.8.1/command.go#L276-L280
// SetArgs sets arguments for the command. It is set to os.Args[1:] by default, if desired, can be overridden
// particularly useful when testing.
func (c *Command) SetArgs(a []string) {
c.args = a
}
And the fix is to explicitly set the command's args to an empty slice to
prevent Cobra from falling back to using `os.Args[1:]` as arguments.
cmd := newSomeThingCommand()
cmd.SetArgs([]string{})
Some tests already take this issue into account, and I updated some tests
for this, but there's likely many other ones that can use the same treatment.
Perhaps the Cobra maintainers would accept a contribution to make their
condition less specific and to look for binaries ending with a `.test`
suffix (which is what compiled binaries usually are named as).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-03 19:29:04 -04:00
|
|
|
cmd.SetErr(io.Discard)
|
2017-03-26 02:23:24 -04:00
|
|
|
cmd.SetArgs([]string{"foo", "bar"})
|
|
|
|
|
2018-03-06 15:54:24 -05:00
|
|
|
assert.Error(t, cmd.Execute(), "Failed to remove some resources from stack: foo")
|
2018-03-05 18:53:52 -05:00
|
|
|
assert.Check(t, is.DeepEqual(allServiceIDs, removedServices))
|
|
|
|
assert.Check(t, is.DeepEqual(allNetworkIDs, cli.removedNetworks))
|
|
|
|
assert.Check(t, is.DeepEqual(allSecretIDs, cli.removedSecrets))
|
|
|
|
assert.Check(t, is.DeepEqual(allConfigIDs, cli.removedConfigs))
|
2017-03-26 02:23:24 -04:00
|
|
|
}
|