projectatomic / vagrant-service-manager

To provide the user a CLI to configure the ADB/CDK for different use cases and to provide glue between ADB/CDK and the user's developer environment.
GNU General Public License v2.0
18 stars 16 forks source link

Acceptance test for --cli-version and --path options and updated help for install-cli #391

Closed coolbrg closed 8 years ago

coolbrg commented 8 years ago

Fix #326

coolbrg commented 8 years ago

@hferentschik , any idea how to test --path option?

Suppose, if I run command vagrant service-manager install-cli docker --path <path>

How do I validate this path in terms of current acceptance test framework?

hferentschik commented 8 years ago

@budhrg let me have a look into it, starting with CI ...

hferentschik commented 8 years ago

@budhrg, could you please push a rebased version of this pull request? This should also trigger the hopefully working CI job again.

coolbrg commented 8 years ago

@hferentschik It looks like rubocop check is not really working locally

➜  vagrant-service-manager git:(fix-326) ✗ bundle exec rake rubocop
Running RuboCop...
Inspecting 47 files
...............................................

47 files inspected, no offenses detected
coolbrg commented 8 years ago

@hferentschik Pushing rebased version will not work. Some "guard related warning" in new changes I committed. Will fix that and push along with rebase.

coolbrg commented 8 years ago

@hferentschik Rubocop version was the culprit :wink:

➜  vagrant-service-manager git:(fix-326) ✗ rubocop -v
0.42.0
➜  vagrant-service-manager git:(fix-326) ✗ rubocop lib/vagrant-service-manager/installer.rb
Inspecting 1 file
.

1 file inspected, no offenses detected
➜  vagrant-service-manager git:(fix-326) ✗ gem update rubocop
Updating installed gems
Updating rubocop
Fetching: rubocop-0.43.0.gem (100%)
Successfully installed rubocop-0.43.0
Parsing documentation for rubocop-0.43.0
Installing ri documentation for rubocop-0.43.0
Installing darkfish documentation for rubocop-0.43.0
Done installing documentation for rubocop after 12 seconds
Parsing documentation for rubocop-0.43.0
Done installing documentation for rubocop after 6 seconds
Gems updated: rubocop
➜  vagrant-service-manager git:(fix-326) ✗ rubocop lib/vagrant-service-manager/installer.rb
Inspecting 1 file
C

Offenses:

lib/vagrant-service-manager/installer.rb:39:9: C: Use a guard clause instead of wrapping the code inside a conditional expression.
        if @options.key?('--path') && !File.exist?(File.dirname(@options['--path']))
        ^^

1 file inspected, 1 offense detected
coolbrg commented 8 years ago

retest this please

coolbrg commented 8 years ago

retest this please

coolbrg commented 8 years ago

@hferentschik Build passing now.

I am unable to add the test for --path option. How you can put path and verify it in aruba test framework?

hferentschik commented 8 years ago

I am unable to add the test for --path option. How you can put path and verify it in aruba test framework?

What exactly is the problem?

coolbrg commented 8 years ago

What exactly is the problem?

What path to specify in command when I say

When I run `bundle exec vagrant service-manager install-cli docker --path <which path>`
Then the exit status should be 0
And the binary "docker" should be installed with path "which path" (how do I verify here?)
hferentschik commented 8 years ago
When I run `bundle exec vagrant service-manager install-cli docker --path #{ENV['VAGRANT_HOME']}/data/service-manager/foo`
Then the exit status should be 0
And the binary "docker" should be installed with path "#{ENV['VAGRANT_HOME']}/data/service-manager/foo" (how do I verify here?)

Otherwise create tmpdir and let it install into the temp dir. IMO not so nice. Better to install into something which stays in the build directory.

hferentschik commented 8 years ago

@budhrg afaict nothing has changed since my last review, or am I missing something?

coolbrg commented 8 years ago

@hferentschik sorry couldn't update this PR as I am still finding the way to write test for --path option.

hferentschik commented 8 years ago

sorry couldn't update this PR as I am still finding the way to write test for --path option.

no worries. For some reason I thought you wanted me to have another look.

coolbrg commented 8 years ago

@hferentschik Updated test for --path option. Used /tmp as of now. Not getting framework related path to use.

coolbrg commented 8 years ago

retest this please

coolbrg commented 8 years ago

@hferentschik Also added documentation in commands.adoc

hferentschik commented 8 years ago

retest

hferentschik commented 8 years ago

Seems that we have Rubocop failures again :-( This time a Rubocop 0.44 got releases. I guess we need to thing about, whether we want to pin the Rubocop version to prevent this to happen or whether we just handle it as it comes. Either way, let's do this separately. Let's create an issue and pull request for upgrading (and potentially pinning version) of Rubocop. Once this is in place we can place this pull request on top. @budhrg Any strong feeling regarding setting a fixed version number vs dealing with new "violations"?

hferentschik commented 8 years ago

@budhrg I am also closing this pull request. Let's split Rubocop from install-cli and create new pull requests. We take it from there.

coolbrg commented 8 years ago

feeling regarding setting a fixed version number

@hferentschik

I am in favor of pinning to fix version and update it later on instead of dealing with new violations every time new version is released.

coolbrg commented 8 years ago

Also, will give the PR of this issue https://github.com/projectatomic/vagrant-service-manager/issues/412 first @hferentschik