Merge lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview into lp:~juju-gui/charms/trusty/juju-gui/trunk

[Uros Jovanovic]

enables ftests on local env

Create local environment (with "local" also being the name of that environment) and ftests can be now be run local as well.

https://codereview.appspot.com/113720043/

[lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview]

188. By Uros Jovanovic on 2014-07-10

enable local to skip the --to 0 param

[Uros Jovanovic (uros-jovanovic)]

Reviewers: mp+226270_code.launchpad.net,

Message:
Please take a look.

Description:
minor change in hacking.md

https://code.launchpad.net/~uros-jovanovic/charms/trusty/juju-gui/hackingreview/+merge/226270

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/113720043/

Affected files (+12, -6 lines):
   M HACKING.md
   A [revision details]
   M tests/20-functional.test
   M tests/deploy.py

Index: HACKING.md
=== modified file 'HACKING.md'
--- HACKING.md 2014-04-22 10:28:39 +0000
+++ HACKING.md 2014-07-10 10:24:19 +0000
@@ -140,10 +140,10 @@
  When something goes wrong, on your local machine run
  `juju debug-hooks juju-gui/0` or similar. This will initially put you on
the
  unit that has the problem. You can look at what is going on in
-`/var/lib/juju/units/[NAME OF UNIT]`. There is a charm.log file to
-investigate, and a charm directory which contains the charm. The charm
-directory contains the `juju-gui` and `juju` directories, so everything you
-need is there.
+`/var/lib/juju/agents/[NAME OF UNIT]` (or instead of agents use
`containers`
+in the local environment. There is a charm.log file to investigate, and a
+charm directory which contains the charm. The charm directory contains the
+`juju-gui` and `juju` directories, so everything you need is there.

  If juju recognized an error (for instance, the unit is in
an "install-error"
  state) then you can do more. In another terminal on your local machine,
run

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: tests/20-functional.test
=== modified file 'tests/20-functional.test'
--- tests/20-functional.test 2014-04-14 17:00:09 +0000
+++ tests/20-functional.test 2014-07-10 11:53:35 +0000
@@ -23,6 +23,7 @@
  import unittest
  import urllib2
  import urlparse
+import os

  from selenium.webdriver import Firefox
  from selenium.webdriver.support import ui
@@ -50,6 +51,10 @@
      print(err)
  bootstrap_node_series = juju_status()['machines']['0']['series']

+forced_machine = 0
+env_par = os.getenv('JUJU_ENV')
+if env_par == "local":
+ forced_machine = None

  def juju_deploy_gui(options=None):
      """Deploy the Juju GUI charm with the given options.
@@ -62,7 +67,7 @@
      service_name = make_service_name(prefix='juju-gui-')
      unit_info = juju_deploy(
          'juju-gui', service_name=service_name, options=options,
- force_machine=0, series=bootstrap_node_series)
+ force_machine=forced_machine, series=bootstrap_node_series)
      return service_name, unit_info

Index: tests/deploy.py
=== modified file 'tests/deploy.py'
--- tests/deploy.py 2014-04-22 09:27:01 +0000
+++ tests/deploy.py 2014-07-10 11:53:35 +0000
@@ -32,7 +32,6 @@
      wait_for_unit,
  )

-
  rsync = command('rsync', '-a',
                  '--exclude', '.git',
                  '--exclude', '.bzr',

[lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview]

189. By Uros Jovanovic on 2014-07-10

added flag to select the local environment

[Uros Jovanovic (uros-jovanovic)]

Please take a look.

https://codereview.appspot.com/113720043/

[Richard Harding (rharding)]

Couple of comments, will let Jay do the final looks good approval based
on successful lxc and ec2 test runs.

https://codereview.appspot.com/113720043/diff/20001/HACKING.md
File HACKING.md (right):

https://codereview.appspot.com/113720043/diff/20001/HACKING.md#newcode107
HACKING.md:107: make ftest JUJU_ENV="myenv" JUJU_ENV_TYPE="local"
The doc is about colocation. Should this be a COLO=false flag instead?

https://codereview.appspot.com/113720043/diff/20001/Makefile
File Makefile (right):

https://codereview.appspot.com/113720043/diff/20001/Makefile#newcode20
Makefile:20: rsync xvfb firefox charm-tools
why is firefox the requirement? Are we not able to assume a browser and
let the dev select?

https://codereview.appspot.com/113720043/

[lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview]

190. By Uros Jovanovic on 2014-07-10

as juju-test do not pass parameters, we need to rely on the name of the environment for local deploy

[Uros Jovanovic (uros-jovanovic)]

Please take a look.

https://codereview.appspot.com/113720043/

[Jay R. Wren (evarlast)]

nits:
line 55 in tests/20... comment refers to JUJU_ENV_TYPE instead of
JUJU_ENV
i don't feel the whitespace change in tests/deploy.py is necessary

https://codereview.appspot.com/113720043/

[lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview]

191. By Uros Jovanovic on 2014-07-10

corrected description for JUJU_ENV

[Uros Jovanovic (uros-jovanovic)]

Please take a look.

https://codereview.appspot.com/113720043/

[Jay R. Wren (evarlast)]

LGTM

https://codereview.appspot.com/113720043/

[Uros Jovanovic (uros-jovanovic)]

*** Submitted:

enables ftests on local env

Create local environment (with "local" also being the name of that
environment) and ftests can be now be run local as well.

R=rharding, jay.wren
CC=
https://codereview.appspot.com/113720043

https://codereview.appspot.com/113720043/

=== modified file 'HACKING.md'
--- HACKING.md	2014-07-09 18:38:56 +0000
+++ HACKING.md	2014-07-10 18:59:52 +0000
@@ -103,6 +103,19 @@
 `~/.juju/environments.yaml`, that will be bootstrapped before running the
 tests and destroyed at the end of the test run.
 
+To run a test on a local environment, create an environment named "local"
+and type "local" like this:
+
+environments.yaml
+
+environments:
+  local:
+    type: local
+
+Then run:
+
+    make ftest JUJU_ENV="local"
+
 ## Running the Charm From Development ##
 
 If you have set up your environment to run your local development charm,
@@ -142,10 +155,10 @@
 When something goes wrong, on your local machine run
 `juju debug-hooks juju-gui/0` or similar.  This will initially put you on the
 unit that has the problem.  You can look at what is going on in
-`/var/lib/juju/units/[NAME OF UNIT]`.  There is a charm.log file to
-investigate, and a charm directory which contains the charm.  The charm
-directory contains the `juju-gui` and `juju` directories, so everything you
-need is there.
+`/var/lib/juju/agents/[NAME OF UNIT]` (or instead of agents use `containers`
+in the local environment. There is a charm.log file to investigate, and a 
+charm directory which contains the charm.  The charm directory contains the 
+`juju-gui` and `juju` directories, so everything you need is there.
 
 If juju recognized an error (for instance, the unit is in an "install-error"
 state) then you can do more.  In another terminal on your local machine, run

=== modified file 'Makefile'
--- Makefile	2014-04-22 09:27:01 +0000
+++ Makefile	2014-07-10 18:59:52 +0000
@@ -17,8 +17,7 @@
 JUJUTEST = yes | juju-test --timeout=60m -v -e "$(JUJU_ENV)"
 VENV = tests/.venv
 SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-virtualenv \
-	rsync xvfb
-
+	rsync xvfb charm-tools
 
 all: setup
 

=== modified file 'tests/20-functional.test'
--- tests/20-functional.test	2014-04-14 17:00:09 +0000
+++ tests/20-functional.test	2014-07-10 18:59:52 +0000
@@ -23,6 +23,7 @@
 import unittest
 import urllib2
 import urlparse
+import os
 
 from selenium.webdriver import Firefox
 from selenium.webdriver.support import ui
@@ -50,6 +51,16 @@
     print(err)
 bootstrap_node_series = juju_status()['machines']['0']['series']
 
+"""
+If JUJU_ENV is defined to local, then the --to is not
+used as it is not needed in the local env. Otherwise, the 
+default placing of the deployed GUI charm is on machine 0 
+with a --to 0
+"""
+forced_machine = 0
+env_par = os.getenv('JUJU_ENV')
+if env_par == "local":
+    forced_machine = None
 
 def juju_deploy_gui(options=None):
     """Deploy the Juju GUI charm with the given options.
@@ -62,7 +73,7 @@
     service_name = make_service_name(prefix='juju-gui-')
     unit_info = juju_deploy(
         'juju-gui', service_name=service_name, options=options,
-        force_machine=0, series=bootstrap_node_series)
+        force_machine=forced_machine, series=bootstrap_node_series)
     return service_name, unit_info
 
 

=== modified file 'tests/deploy.py'
--- tests/deploy.py	2014-04-22 09:27:01 +0000
+++ tests/deploy.py	2014-07-10 18:59:52 +0000
@@ -32,7 +32,6 @@
     wait_for_unit,
 )
 
-
 rsync = command('rsync', '-a',
                 '--exclude', '.git',
                 '--exclude', '.bzr',