search

juju / juju-gui-charm

Charm for Juju GUI.
GNU Affero General Public License v3.0
2 stars 16 forks source link

Fix for when pip -> pip3 #33

Closed merlijn-sebrechts closed 8 years ago

merlijn-sebrechts commented 8 years ago

When hulk-smashing this charm onto the same host of a reactive charm, juju-gui fails to deploy because pip points to pip3. This fix fixes that

jujugui commented 8 years ago

please test this

merlijn-sebrechts commented 8 years ago

Hi

I suspect you are asking me to run the functional tests against my patch? The HACKING.md is out of date so I have no idea if I'm doing this right.

When I run the tests against the master branch without my patch, it fails: 

juju-test.conductor DEBUG   : Waiting for bootstrap
juju-test.conductor DEBUG   : Still not bootstrapped
juju-test.conductor DEBUG   : Running the following: juju status -e msebrechts
juju-test.conductor DEBUG   : State for 1.25.3: started
juju-test.conductor.20-functional.test DEBUG   : Running 20-functional.test (tests/20-functional.test)
test_deployer (__main__.TestBuiltinServerLocalRelease) ... skipped 'admin secret was not found'
test_deployer_invalid_bundle_name (__main__.TestBuiltinServerLocalRelease) ... skipped 'admin secret was not found'
test_deployer_invalid_bundle_yaml (__main__.TestBuiltinServerLocalRelease) ... skipped 'admin secret was not found'
test_deployer_not_authenticated (__main__.TestBuiltinServerLocalRelease) ... ERROR

======================================================================
ERROR: test_deployer_not_authenticated (__main__.TestBuiltinServerLocalRelease)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/20-functional.test", line 105, in setUp
    selenium = self.selenium = Firefox()
  File "/home/merlijn/TEMP/juju-gui-charm-develop/tests/.venv/local/lib/python2.7/site-packages/selenium/webdriver/firefox/webdriver.py", line 59, in __init__
    self.binary, timeout),
  File "/home/merlijn/TEMP/juju-gui-charm-develop/tests/.venv/local/lib/python2.7/site-packages/selenium/webdriver/firefox/extension_connection.py", line 47, in __init__
    self.binary.launch_browser(self.profile)
  File "/home/merlijn/TEMP/juju-gui-charm-develop/tests/.venv/local/lib/python2.7/site-packages/selenium/webdriver/firefox/firefox_binary.py", line 66, in launch_browser
    self._wait_until_connectable()
  File "/home/merlijn/TEMP/juju-gui-charm-develop/tests/.venv/local/lib/python2.7/site-packages/selenium/webdriver/firefox/firefox_binary.py", line 105, in _wait_until_connectable
    raise WebDriverException("Can't load the profile. Profile "
WebDriverException: Message: Can't load the profile. Profile Dir: %s If you specified a log_file in the FirefoxBinary constructor, check it for details.

----------------------------------------------------------------------
Ran 4 tests in 213.962s

FAILED (errors=1, skipped=3)
juju-test.conductor.20-functional.test DEBUG   : Attribute admin-secret not found

juju-test.conductor.20-functional.test DEBUG   : Got exit code: 1
juju-test.conductor.20-functional.test RESULT  : ✘
juju-test.conductor DEBUG   : Tearing down msebrechts juju environment
juju-test.conductor DEBUG   : Calling "juju destroy-environment -y msebrechts"
juju-test INFO    : Results: 0 passed, 1 failed, 0 errored
Makefile:59: recipe for target 'ftest' failed
make: *** [ftest] Error 1

What I did:

make
make test JUJU_ENV="msebrechts"

Ubuntu 15.10

Is there something wrong with my machine, am I doing something wrong?

jrwren commented 8 years ago

This change breaks the install on trusty.)

There is no pip2 in path on trusty.)

Update: Well, don't mind me, this is totally wrong. pip2 is there are part of python-pip package in trusty.

This charm currently targets trusty. When it gets xenial compatibility it will still need trusty compatibility.

Thanks for the MR. We can't accept it if it breaks for trusty.

:+1:

merlijn-sebrechts commented 8 years ago

Strange, pip2 is in path on my 14.04 systems. Anyway, I changed it to the hardcoded pip2 path.

bac commented 8 years ago

Hi @galgalesh thanks for the fix. I'm just popping in to say the terse comment "please test this" was not directed at you but at our CI system.

merlijn-sebrechts commented 8 years ago

Do I need to make more changes or can this be merged?

jcsackett commented 8 years ago

Hi @galgalesh.

Sorry for the delays on this. I've kicked off a CI run on this now, and will QA it soon; in the meantime could you provide me an example charm that triggered this failure so we can QA the fix against that scenario?

jcsackett commented 8 years ago

@galgalesh it looks like there's a test failure http://ci.jujugui.org:8080/job/juju-gui-charm/72/console

It's in the unittests for the charm (make unittest); I've verified that the same test fails on a local trusty machine of mine.

Looks like you just need to update the pip call in the test to match your changes.

merlijn-sebrechts commented 8 years ago

This is an example charm to trigger the scenario. Deploy this, then hulk-smash juju-gui onto the node

https://github.com/galgalesh/layered-charm

jcsackett commented 8 years ago

Thanks @galgalesh.

QA notes:

On a trusty environment, I deployed the layered-charm, then deployed juju-gui to the same machine. It failed as expected.

I tore down the env, bootstrapped a new one, and using this branch I repeated the above steps. The charm deployed properly.

QA :ok:

@galgalesh if you can make the changes to address the issue above (https://github.com/juju/juju-gui-charm/pull/33#issuecomment-191811989) I think we can land this for you.

merlijn-sebrechts commented 8 years ago

Done

jcsackett commented 8 years ago

We have a passing test run at http://ci.jujugui.org:8080/job/juju-gui-charm/73/

:+1:

:shipit:

jcsackett commented 8 years ago

Thanks for all your work @galgalesh!

jujugui commented 8 years ago

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-charm-merge