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

Remove cli test for duplicate --net/--network opts #42163

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Mar 18, 2021

This seems to be testing a strange case, specifically that one can set
the --net and --network in the same command with the same network.

Indeed this used to work with older CLIs but newer ones error out when
validating the request before sending it to the daemon.

Opening this for discussion because:

  1. This doesn't seem to be testing anything at all related to the rest
    of the test
  2. Not really providing any value here.
  3. Is testing that a technically invalid option is successful (whether
    the option should be valid as it relates to the CLI accepting it is
    debatable).
  4. Such a case seems fringe and even a bug in whatever is calling the
    CLI with such options.

The change that caused this is from docker/cli#1767

@cpuguy83 cpuguy83 requested a review from tiborvass March 18, 2021 17:14
This seems to be testing a strange case, specifically that one can set
the `--net` and `--network` in the same command with the same network.

Indeed this used to work with older CLIs but newer ones error out when
validating the request before sending it to the daemon.

Opening this for discussion because:

1. This doesn't seem to be testing anything at all related to the rest
   of the test
2. Not really providing any value here.
3. Is testing that a technically invalid option is successful (whether
   the option should be valid as it relates to the CLI accepting it is
   debatable).
4. Such a case seems fringe and even a bug in whatever is calling the
   CLI with such options.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants