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

Proposed by Uros Jovanovic
Status: Merged
Merged at revision: 187
Proposed branch: lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview
Merge into: lp:~juju-gui/charms/trusty/juju-gui/trunk
Diff against target: 102 lines (+30/-8)
4 files modified
HACKING.md (+17/-4)
Makefile (+1/-2)
tests/20-functional.test (+12/-1)
tests/deploy.py (+0/-1)
To merge this branch: bzr merge lp:~uros-jovanovic/charms/trusty/juju-gui/hackingreview
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+226270@code.launchpad.net

Description of the change

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/

To post a comment you must log in.
188. By Uros Jovanovic

enable local to skip the --to 0 param

Revision history for this message
Uros Jovanovic (uros-jovanovic) wrote :

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',

189. By Uros Jovanovic

added flag to select the local environment

Revision history for this message
Uros Jovanovic (uros-jovanovic) wrote :
Revision history for this message
Richard Harding (rharding) wrote :

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/

190. By Uros Jovanovic

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

Revision history for this message
Uros Jovanovic (uros-jovanovic) wrote :
Revision history for this message
Jay R. Wren (evarlast) wrote :

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/

191. By Uros Jovanovic

corrected description for JUJU_ENV

Revision history for this message
Uros Jovanovic (uros-jovanovic) wrote :
Revision history for this message
Jay R. Wren (evarlast) wrote :
Revision history for this message
Uros Jovanovic (uros-jovanovic) wrote :

*** 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/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
103`~/.juju/environments.yaml`, that will be bootstrapped before running the103`~/.juju/environments.yaml`, that will be bootstrapped before running the
104tests and destroyed at the end of the test run.104tests and destroyed at the end of the test run.
105105
106To run a test on a local environment, create an environment named "local"
107and type "local" like this:
108
109environments.yaml
110
111environments:
112 local:
113 type: local
114
115Then run:
116
117 make ftest JUJU_ENV="local"
118
106## Running the Charm From Development ##119## Running the Charm From Development ##
107120
108If you have set up your environment to run your local development charm,121If you have set up your environment to run your local development charm,
@@ -142,10 +155,10 @@
142When something goes wrong, on your local machine run155When something goes wrong, on your local machine run
143`juju debug-hooks juju-gui/0` or similar. This will initially put you on the156`juju debug-hooks juju-gui/0` or similar. This will initially put you on the
144unit that has the problem. You can look at what is going on in157unit that has the problem. You can look at what is going on in
145`/var/lib/juju/units/[NAME OF UNIT]`. There is a charm.log file to158`/var/lib/juju/agents/[NAME OF UNIT]` (or instead of agents use `containers`
146investigate, and a charm directory which contains the charm. The charm159in the local environment. There is a charm.log file to investigate, and a
147directory contains the `juju-gui` and `juju` directories, so everything you160charm directory which contains the charm. The charm directory contains the
148need is there.161`juju-gui` and `juju` directories, so everything you need is there.
149162
150If juju recognized an error (for instance, the unit is in an "install-error"163If juju recognized an error (for instance, the unit is in an "install-error"
151state) then you can do more. In another terminal on your local machine, run164state) then you can do more. In another terminal on your local machine, run
152165
=== modified file 'Makefile'
--- Makefile 2014-04-22 09:27:01 +0000
+++ Makefile 2014-07-10 18:59:52 +0000
@@ -17,8 +17,7 @@
17JUJUTEST = yes | juju-test --timeout=60m -v -e "$(JUJU_ENV)"17JUJUTEST = yes | juju-test --timeout=60m -v -e "$(JUJU_ENV)"
18VENV = tests/.venv18VENV = tests/.venv
19SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-virtualenv \19SYSDEPS = build-essential bzr libapt-pkg-dev libpython-dev python-virtualenv \
20 rsync xvfb20 rsync xvfb charm-tools
21
2221
23all: setup22all: setup
2423
2524
=== 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 @@
23import unittest23import unittest
24import urllib224import urllib2
25import urlparse25import urlparse
26import os
2627
27from selenium.webdriver import Firefox28from selenium.webdriver import Firefox
28from selenium.webdriver.support import ui29from selenium.webdriver.support import ui
@@ -50,6 +51,16 @@
50 print(err)51 print(err)
51bootstrap_node_series = juju_status()['machines']['0']['series']52bootstrap_node_series = juju_status()['machines']['0']['series']
5253
54"""
55If JUJU_ENV is defined to local, then the --to is not
56used as it is not needed in the local env. Otherwise, the
57default placing of the deployed GUI charm is on machine 0
58with a --to 0
59"""
60forced_machine = 0
61env_par = os.getenv('JUJU_ENV')
62if env_par == "local":
63 forced_machine = None
5364
54def juju_deploy_gui(options=None):65def juju_deploy_gui(options=None):
55 """Deploy the Juju GUI charm with the given options.66 """Deploy the Juju GUI charm with the given options.
@@ -62,7 +73,7 @@
62 service_name = make_service_name(prefix='juju-gui-')73 service_name = make_service_name(prefix='juju-gui-')
63 unit_info = juju_deploy(74 unit_info = juju_deploy(
64 'juju-gui', service_name=service_name, options=options,75 'juju-gui', service_name=service_name, options=options,
65 force_machine=0, series=bootstrap_node_series)76 force_machine=forced_machine, series=bootstrap_node_series)
66 return service_name, unit_info77 return service_name, unit_info
6778
6879
6980
=== 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 @@
32 wait_for_unit,32 wait_for_unit,
33)33)
3434
35
36rsync = command('rsync', '-a',35rsync = command('rsync', '-a',
37 '--exclude', '.git',36 '--exclude', '.git',
38 '--exclude', '.bzr',37 '--exclude', '.bzr',

Subscribers

People subscribed via source and target branches