Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(32)

Issue 26530043: Improve charm test venv creation.

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 3 months ago by frankban
Modified:
7 years, 3 months ago
Reviewers:
mp+195199, gary.poster
Visibility:
Public.

Description

Improve charm test venv creation. Also added missing SYSDEP. https://code.launchpad.net/~frankban/charms/precise/juju-gui/venv-fixes/+merge/195199 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Improve charm test venv creation. #

Total comments: 2

Patch Set 3 : Improve charm test venv creation. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -5 lines) Patch
M Makefile View 1 chunk +3 lines, -2 lines 0 comments Download
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M revision View 1 1 chunk +1 line, -1 line 0 comments Download
M tests/00-setup View 1 2 1 chunk +5 lines, -2 lines 0 comments Download

Messages

Total messages: 4
frankban
Please take a look.
7 years, 3 months ago (2013-11-14 10:57:29 UTC) #1
frankban
Please take a look.
7 years, 3 months ago (2013-11-14 11:25:54 UTC) #2
gary.poster
LGTM with consideration of comment. https://codereview.appspot.com/26530043/diff/20001/tests/00-setup File tests/00-setup (right): https://codereview.appspot.com/26530043/diff/20001/tests/00-setup#newcode35 tests/00-setup:35: [ $retcode -eq 0 ...
7 years, 3 months ago (2013-11-14 11:57:56 UTC) #3
frankban
7 years, 3 months ago (2013-11-14 12:34:28 UTC) #4
*** Submitted:

Improve charm test venv creation.

Also added missing SYSDEP.

R=gary.poster
CC=
https://codereview.appspot.com/26530043

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup
File tests/00-setup (right):

https://codereview.appspot.com/26530043/diff/20001/tests/00-setup#newcode35
tests/00-setup:35: [ $retcode -eq 0 ] && touch $ACTIVATE || rm -f $ACTIVATE
On 2013/11/14 11:57:56, gary.poster wrote:
> The only downside to this is that, since the $ACTIVATE is removed, you can't
use
> it to diagnose issues.  What would you think of touching $TEST_REQUIREMENTS in
> the failure case, instead?

Great suggestion! Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b