Skip to content
This repository has been archived by the owner on Jul 29, 2018. It is now read-only.

Implement the --cli-version and --path options for install-cli #326

Closed
hferentschik opened this issue Jul 13, 2016 · 6 comments
Closed

Implement the --cli-version and --path options for install-cli #326

hferentschik opened this issue Jul 13, 2016 · 6 comments
Labels
Milestone

Comments

@hferentschik
Copy link
Contributor

These options are not properly implemented yet. The help does not mention them, but they are mentioned in the README (going to remove it together with the Cucumber test which does not seem to do anything).

@brgnepal
Copy link
Contributor

@hferentschik --cli-version and --path options are implemented in code here. Unit tests are implemented using --cli-versionoption only. Cucumber test one negative test here. I can add one more for positive test of installing docker with --cli-version if needed.

@hferentschik
Copy link
Contributor Author

--cli-version and --path options are implemented in code here.

I've seen that. But it does not mean that it works, right? When I tried to use the option, all I kept getting was the help message that I used the command wrong. Note, this help message does not even list the --install-cli and --path.

Unit tests are implemented using --cli-versionoption only.

Well, this might work on the ADBDockerBinaryHandler level as a unit, but it does not mean and prove the options are fully exposed end to end. Hence we should have acceptance tests for all top level commands. ATM, we don't.

Cucumber test one negative test here.

This test does not mean anything. My guess is that it returns 126, because it thinks it is an invalid option. This test needs to verify the exit code AND something else, for example that the error message is "invalid version for docker". Then I would believe the test. In fact I removed the test for now. These tests are dangerous, because they give you a feeling of misguided safety. The assertion must be specific enough to just fail/pass the condition I am testing.

I can add one more for positive test of installing docker with --cli-version if needed.

+1. If you can do that and it works without any further code changes I take everything back.

@brgnepal brgnepal modified the milestones: v1.4.0, v1.3.0 Aug 12, 2016
@hferentschik
Copy link
Contributor Author

@budhrg what's the state here? Do this options exist or not?

@brgnepal
Copy link
Contributor

@hferentschik Documentation is remaining here. Will work with @Preeticp

@hferentschik
Copy link
Contributor Author

What's about integration tests. Last time I checked there were none.

@hferentschik
Copy link
Contributor Author

Merged via pull request #414

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants