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

Fix #376 --path option of install cli not working as expected #416

Closed coolbrg closed 7 years ago

coolbrg commented 7 years ago

Fix #376

coolbrg commented 7 years ago

@hferentschik CI is passing :+1:

coolbrg commented 7 years ago

retest this please

coolbrg commented 7 years ago

@hferentschik The tests seems to be passing now. This time these are passing genuinely :smile:. However, there is one testcase which is failing in terms of openshift bundle exec vagrant service-manager install-cli openshift which I don't know. I tried a lot for its debugging.

However, its cli-version bundle exec vagrant service-manager install-cli openshift --cli-version 1.3.1 is passing. This PR has --cli-version.

The failed one is here https://ci.centos.org/job/vagrant-service-manager-budh/11/console.

However, no such issue in case of cdk-openshift.feature (https://github.com/budhrg/vagrant-service-manager/blob/e81d07a3e4b73042a1f633391318696e6e7750f0/features/cdk-openshift.feature).

I believe if you have not any other review, we can merge it and investigate the edge case in this issue https://github.com/projectatomic/vagrant-service-manager/issues/415

Let me know what is your opinion.

coolbrg commented 7 years ago

retest this please

hferentschik commented 7 years ago

retest

hferentschik commented 7 years ago

retest

hferentschik commented 7 years ago

test this please

coolbrg commented 7 years ago

@hferentschik Hi, I have updated the tests now. It include both evalauable the install-cli command and added step for sleep too.

coolbrg commented 7 years ago

retest this please

hferentschik commented 7 years ago

test this please

hferentschik commented 7 years ago

@budhrg Did you create an issue somewhere in adb to find out why we need a wait period before between 'vagrant up' and the service-manager 'status'? That does not seem right to me.

Also, lets create a follow up issue in service-manager to find a more direct condition than sleeping 10 seconds. This might or might not work depending on the machine the tests are running. It could be something like "And wait max x seconds to OpenShift to be operational" where the implementation checks in a loop some condition of the OpenShift cluster and times out after max seconds. Adding random waits to a test is a bad smell.

hferentschik commented 7 years ago

@budhrg - does this bundle exec rake clean features BOX=adb,cdk pass all tests on your work station?

hferentschik commented 7 years ago

@budhrg Didn't we want to enable the Kubernates features on CI as well?

hferentschik commented 7 years ago

@budhrg For what it's worth bundle exec rake clean features BOX=adb,cdk seems to work for me now ignoring the string conversion error (note this in itself makes of course all adb openshift tests to be skipped)