Skip to content
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

cli/command: remove dot-imports and unhandled errors, and fix TestSwarmUpdate #4619

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Oct 23, 2023

swarm: TestSwarmUpdate: remove non-existing "--quiet" flag

The docker swarm update copmmand does not have a --quiet flag, but this
test was trying to set it.

docker swarm update --help

Usage:  docker swarm update [OPTIONS]

Update the swarm

Options:
      --autolock                        Change manager autolocking setting (true|false)
      --cert-expiry duration            Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
      --dispatcher-heartbeat duration   Dispatcher heartbeat period (ns|us|ms|s|m|h) (default 5s)
      --external-ca external-ca         Specifications of one or more certificate signing endpoints
      --max-snapshots uint              Number of additional Raft snapshots to retain
      --snapshot-interval uint          Number of log entries between Raft snapshots (default 10000)
      --task-history-limit int          Task history retention limit (default 5)

The test didn't catch this issue, because errors when setting the flag were
not handled, so also adding error-handling;

=== Failed
=== FAIL: cli/command/swarm TestSwarmUpdate (0.00s)
    update_test.go:177: assertion failed: error is not nil: no such flag -quiet

cli/command: remove dot-imports and unhandled errors

Please the linters in preparation of updating golangci-lint;

  • remove dot-imports

  • add some checks for unhandled errors

  • replace some fixed-value variables for consts

    cli/command/image/build/context.go:238:17: G107: Potential HTTP request made with variable url (gosec)
    if resp, err = http.Get(url); err != nil {
    ^
    cli/command/idresolver/idresolver_test.go:7:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/registry_test.go:7:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/cli/command" // Prevents a circular import with "github.com/docker/cli/internal/test"
    ^
    cli/command/task/print_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/swarm/update_test.go:10:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/swarm/unlock_key_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/swarm/join_token_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/node/list_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/node/promote_test.go:8:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/node/demote_test.go:8:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package functions
    ^
    cli/command/node/ps_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/node/update_test.go:8:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/node/inspect_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package functions
    ^
    cli/command/secret/ls_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/secret/inspect_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/volume/inspect_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/volume/list_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/config/inspect_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/config/ls_test.go:11:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/network/list_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders"
    ^
    cli/command/container/list_test.go:10:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/service/list_test.go:12:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders"
    ^
    cli/command/service/client_test.go:6:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/stack/list_test.go:8:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/stack/services_test.go:9:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^
    cli/command/stack/ps_test.go:10:2: dot-imports: should not use dot imports (revive)
    . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
    ^

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

. "github.com/docker/cli/cli/command" // Prevents a circular import with "github.com/docker/cli/internal/test"
"github.com/docker/cli/cli/command"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only one that's different; hope it works without.

@thaJeztah
Copy link
Member Author

thaJeztah commented Oct 23, 2023

Looks like the new error-checks found one issue;

87.09 === Failed
87.09 === FAIL: cli/command/swarm TestSwarmUpdate (0.00s)
87.09     update_test.go:177: assertion failed: error is not nil: no such flag -quiet

Which looks legit, because docker swarm update does not have a --quiet flag;

docker swarm update --help

Usage:  docker swarm update [OPTIONS]

Update the swarm

Options:
      --autolock                        Change manager autolocking setting (true|false)
      --cert-expiry duration            Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
      --dispatcher-heartbeat duration   Dispatcher heartbeat period (ns|us|ms|s|m|h) (default 5s)
      --external-ca external-ca         Specifications of one or more certificate signing endpoints
      --max-snapshots uint              Number of additional Raft snapshots to retain
      --snapshot-interval uint          Number of log entries between Raft snapshots (default 10000)
      --task-history-limit int          Task history retention limit (default 5)

The `docker swarm update` copmmand does not have a `--quiet` flag, but this
test was trying to set it.

    docker swarm update --help

    Usage:  docker swarm update [OPTIONS]

    Update the swarm

    Options:
          --autolock                        Change manager autolocking setting (true|false)
          --cert-expiry duration            Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
          --dispatcher-heartbeat duration   Dispatcher heartbeat period (ns|us|ms|s|m|h) (default 5s)
          --external-ca external-ca         Specifications of one or more certificate signing endpoints
          --max-snapshots uint              Number of additional Raft snapshots to retain
          --snapshot-interval uint          Number of log entries between Raft snapshots (default 10000)
          --task-history-limit int          Task history retention limit (default 5)

The test didn't catch this issue, because errors when setting the flag were
not handled, so also adding error-handling;

    === Failed
    === FAIL: cli/command/swarm TestSwarmUpdate (0.00s)
        update_test.go:177: assertion failed: error is not nil: no such flag -quiet

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Please the linters in preparation of updating golangci-lint;

- remove dot-imports
- add some checks for unhandled errors
- replace some fixed-value variables for consts

    cli/command/image/build/context.go:238:17: G107: Potential HTTP request made with variable url (gosec)
        if resp, err = http.Get(url); err != nil {
                       ^
    cli/command/idresolver/idresolver_test.go:7:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/registry_test.go:7:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/cli/command" // Prevents a circular import with "github.com/docker/cli/internal/test"
        ^
    cli/command/task/print_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/swarm/update_test.go:10:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/swarm/unlock_key_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/swarm/join_token_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/node/list_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/node/promote_test.go:8:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/node/demote_test.go:8:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package functions
        ^
    cli/command/node/ps_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/node/update_test.go:8:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/node/inspect_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package functions
        ^
    cli/command/secret/ls_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/secret/inspect_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/volume/inspect_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/volume/list_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/config/inspect_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/config/ls_test.go:11:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/network/list_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders"
        ^
    cli/command/container/list_test.go:10:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/service/list_test.go:12:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders"
        ^
    cli/command/service/client_test.go:6:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/stack/list_test.go:8:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/stack/services_test.go:9:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^
    cli/command/stack/ps_test.go:10:2: dot-imports: should not use dot imports (revive)
        . "github.com/docker/cli/internal/test/builders" // Import builders to get the builder function as package function
        ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title cli/command: remove dot-imports and unhandled errors cli/command: remove dot-imports and unhandled errors, and fix TestSwarmUpdate Oct 23, 2023
@thaJeztah thaJeztah marked this pull request as ready for review October 23, 2023 14:00
@thaJeztah thaJeztah mentioned this pull request Oct 23, 2023
1 task
@thaJeztah thaJeztah merged commit 8bf53ab into docker:master Oct 24, 2023
74 checks passed
@thaJeztah thaJeztah deleted the nodot branch October 24, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants