Merge lp:~harlowja/cloud-init/tox-venvs into lp:~cloud-init-dev/cloud-init/trunk

[Joshua Harlow]

Use a venv for testing and for lint checking

[Scott Moser (smoser)]

Thanks. this is really nice.

need to be able to run 'make test' without tox. Ie, similar to in openstack using only packaged versions. This is important to be able to run tests on build (which won't have access to stuff other than the archive).

you replaced pep8 and pylint with 'flake8'. I think I'm ok with that, but we have lots of '# pylint:' things throughout the code. if we're dropping that we might as well clean them out. I think thats fine as a separate commit though.

[Joshua Harlow (harlowja)]

Makes sense, I understand the need for both, will make a few tweaks to do that.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:934
https://server-team-jenkins.canonical.com/job/cloud-init-ci/57/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/lp-vote-on-merge/31/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/cloud-init-ci/57/rebuild

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

marking this merged.
we use tox in trunk now.
if you think there is something missing, please propose a merge to cloud-init git.
thanks.

=== modified file 'HACKING.rst'
--- HACKING.rst	2013-02-05 07:10:36 +0000
+++ HACKING.rst	2014-01-26 01:03:26 +0000
@@ -19,9 +19,9 @@
 
   - ``bzr commit``
 
-* Check pylint and pep8 and test, and address any issues:
+* Check flake8 and test, and address any issues:
 
-  - ``make test pylint pep8``
+  - ``make test flake8``
 
 * Push to launchpad to a personal branch:
 

=== modified file 'Makefile'
--- Makefile	2014-01-20 18:50:04 +0000
+++ Makefile	2014-01-26 01:03:26 +0000
@@ -2,40 +2,27 @@
 PY_FILES=$(shell find cloudinit bin tests tools -name "*.py" -type f )
 PY_FILES+="bin/cloud-init"
 
+PY_ENV=$(shell python -c 'import sys; print("py%s%s" % (sys.version_info[0:2]))')
+
 YAML_FILES=$(shell find cloudinit bin tests tools -name "*.yaml" -type f )
 YAML_FILES+=$(shell find doc/examples -name "cloud-config*.txt" -type f )
 
 CHANGELOG_VERSION=$(shell $(CWD)/tools/read-version)
 CODE_VERSION=$(shell python -c "from cloudinit import version; print version.version_string()")
 
-PIP_INSTALL := pip install
-
 ifeq ($(distro),)
   distro = redhat
 endif
 
 all: test check_version
 
-pep8:
-	@$(CWD)/tools/run-pep8 $(PY_FILES)
-
-pylint:
-	@$(CWD)/tools/run-pylint $(PY_FILES)
-
-pyflakes:
-	pyflakes $(PY_FILES)
-
-pip-requirements:
-	@echo "Installing cloud-init dependencies..."
-	$(PIP_INSTALL) -r "$@.txt" -q
-
-pip-test-requirements:
-	@echo "Installing cloud-init test dependencies..."
-	$(PIP_INSTALL) -r "$@.txt" -q
-
-test: clean_pyc
-	@echo "Running tests..."
-	@nosetests $(noseopts) tests/
+flake8:
+	@echo "Running flake8 using tox..."
+	@tox -e flake8 $(PY_FILES)
+
+test:
+	@echo "Running tests for env=$(PY_ENV) using tox..."
+	@tox -e $(PY_ENV)
 
 check_version:
 	@if [ "$(CHANGELOG_VERSION)" != "$(CODE_VERSION)" ]; then \
@@ -44,13 +31,13 @@
 	    else true; fi
 
 clean_pyc:
-	@find . -type f -name "*.pyc" -delete
+	find . -type f -name "*.pyc" -delete
 
 2to3:
 	2to3 $(PY_FILES)
 
 clean: clean_pyc
-	rm -rf /var/log/cloud-init.log /var/lib/cloud/
+	rm -rf /var/log/cloud-init.log /var/lib/cloud/ $(CWD)/*.egg-info $(CWD)/.tox
 
 yaml:
 	@$(CWD)/tools/validate-yaml.py $(YAML_FILES)
@@ -61,5 +48,4 @@
 deb:
 	./packages/bddeb
 
-.PHONY: test pylint pyflakes 2to3 clean pep8 rpm deb yaml check_version
-.PHONY: pip-test-requirements pip-requirements clean_pyc
+.PHONY: test flake8 2to3 clean rpm deb yaml check_version clean_pyc

=== removed file 'pylintrc'
--- pylintrc	2012-10-28 02:25:48 +0000
+++ pylintrc	1970-01-01 00:00:00 +0000
@@ -1,19 +0,0 @@
-[General]
-init-hook='import sys; sys.path.append("tests/")'
-
-[MESSAGES CONTROL]
-# See: http://pylint-messages.wikidot.com/all-codes
-# W0142: *args and **kwargs are fine.
-# W0511: TODOs in code comments are fine.
-# W0702: No exception type(s) specified
-# W0703: Catch "Exception"
-# C0103: Invalid name
-# C0111: Missing docstring
-disable=W0142,W0511,W0702,W0703,C0103,C0111
-
-[REPORTS]
-reports=no
-include-ids=yes
-
-[FORMAT]
-max-line-length=79

=== modified file 'test-requirements.txt'
--- test-requirements.txt	2014-01-18 07:46:19 +0000
+++ test-requirements.txt	2014-01-26 01:03:26 +0000
@@ -1,6 +1,4 @@
 httpretty>=0.7.1
 mocker
 nose
-pep8
-pyflakes
-pylint
+hacking>=0.8.0,<0.9

=== removed file 'tools/hacking.py'
--- tools/hacking.py	2012-11-11 03:32:49 +0000
+++ tools/hacking.py	1970-01-01 00:00:00 +0000
@@ -1,170 +0,0 @@
-#!/usr/bin/env python
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright (c) 2012, Cloudscaling
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""cloudinit HACKING file compliance testing (based off of nova hacking.py)
-
-built on top of pep8.py
-"""
-
-import inspect
-import logging
-import re
-import sys
-
-import pep8
-
-# Don't need this for testing
-logging.disable('LOG')
-
-# N1xx comments
-# N2xx except
-# N3xx imports
-# N4xx docstrings
-# N[5-9]XX (future use)
-
-DOCSTRING_TRIPLE = ['"""', "'''"]
-VERBOSE_MISSING_IMPORT = False
-_missingImport = set([])
-
-
-def import_normalize(line):
-    # convert "from x import y" to "import x.y"
-    # handle "from x import y as z" to "import x.y as z"
-    split_line = line.split()
-    if (line.startswith("from ") and "," not in line and
-           split_line[2] == "import" and split_line[3] != "*" and
-           split_line[1] != "__future__" and
-           (len(split_line) == 4 or
-           (len(split_line) == 6 and split_line[4] == "as"))):
-        return "import %s.%s" % (split_line[1], split_line[3])
-    else:
-        return line
-
-
-def cloud_import_alphabetical(physical_line, line_number, lines):
-    """Check for imports in alphabetical order.
-
-    HACKING guide recommendation for imports:
-    imports in human alphabetical order
-    N306
-    """
-    # handle import x
-    # use .lower since capitalization shouldn't dictate order
-    split_line = import_normalize(physical_line.strip()).lower().split()
-    split_previous = import_normalize(lines[line_number - 2])
-    split_previous = split_previous.strip().lower().split()
-    # with or without "as y"
-    length = [2, 4]
-    if (len(split_line) in length and len(split_previous) in length and
-        split_line[0] == "import" and split_previous[0] == "import"):
-        if split_line[1] < split_previous[1]:
-            return (0, "N306: imports not in alphabetical order (%s, %s)"
-                % (split_previous[1], split_line[1]))
-
-
-def cloud_docstring_start_space(physical_line):
-    """Check for docstring not start with space.
-
-    HACKING guide recommendation for docstring:
-    Docstring should not start with space
-    N401
-    """
-    pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE])  # start
-    if (pos != -1 and len(physical_line) > pos + 1):
-        if (physical_line[pos + 3] == ' '):
-            return (pos, "N401: one line docstring should not start with"
-                " a space")
-
-
-def cloud_todo_format(physical_line):
-    """Check for 'TODO()'.
-
-    HACKING guide recommendation for TODO:
-    Include your name with TODOs as in "#TODO(termie)"
-    N101
-    """
-    pos = physical_line.find('TODO')
-    pos1 = physical_line.find('TODO(')
-    pos2 = physical_line.find('#')  # make sure it's a comment
-    if (pos != pos1 and pos2 >= 0 and pos2 < pos):
-        return pos, "N101: Use TODO(NAME)"
-
-
-def cloud_docstring_one_line(physical_line):
-    """Check one line docstring end.
-
-    HACKING guide recommendation for one line docstring:
-    A one line docstring looks like this and ends in a period.
-    N402
-    """
-    pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE])  # start
-    end = max([physical_line[-4:-1] == i for i in DOCSTRING_TRIPLE])  # end
-    if (pos != -1 and end and len(physical_line) > pos + 4):
-        if (physical_line[-5] != '.'):
-            return pos, "N402: one line docstring needs a period"
-
-
-def cloud_docstring_multiline_end(physical_line):
-    """Check multi line docstring end.
-
-    HACKING guide recommendation for docstring:
-    Docstring should end on a new line
-    N403
-    """
-    pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE])  # start
-    if (pos != -1 and len(physical_line) == pos):
-        print physical_line
-        if (physical_line[pos + 3] == ' '):
-            return (pos, "N403: multi line docstring end on new line")
-
-
-current_file = ""
-
-
-def readlines(filename):
-    """Record the current file being tested."""
-    pep8.current_file = filename
-    return open(filename).readlines()
-
-
-def add_cloud():
-    """Monkey patch pep8 for cloud-init guidelines.
-
-    Look for functions that start with cloud_
-    and add them to pep8 module.
-
-    Assumes you know how to write pep8.py checks
-    """
-    for name, function in globals().items():
-        if not inspect.isfunction(function):
-            continue
-        if name.startswith("cloud_"):
-            exec("pep8.%s = %s" % (name, name))  # pylint: disable=W0122
-
-if __name__ == "__main__":
-    # NOVA based 'hacking.py' error codes start with an N
-    pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}')
-    add_cloud()
-    pep8.current_file = current_file
-    pep8.readlines = readlines
-    try:
-        pep8._main()  # pylint: disable=W0212
-    finally:
-        if len(_missingImport) > 0:
-            print >> sys.stderr, ("%i imports missing in this test environment"
-                    % len(_missingImport))

=== removed file 'tools/run-pep8'
--- tools/run-pep8	2014-01-24 19:47:28 +0000
+++ tools/run-pep8	1970-01-01 00:00:00 +0000
@@ -1,38 +0,0 @@
-#!/bin/bash
-
-if [ $# -eq 0 ]; then
-   files=( bin/cloud-init $(find * -name "*.py" -type f) )
-else
-   files=( "$@" );
-fi
-
-if [ -f 'hacking.py' ]
-then
-    base=`pwd`
-else
-    base=`pwd`/tools/
-fi
-
-IGNORE="E501" # Line too long (these are caught by pylint)
-
-# King Arthur: Be quiet! ... Be Quiet! I Order You to Be Quiet.
-IGNORE="$IGNORE,E121" # Continuation line indentation is not a multiple of four
-IGNORE="$IGNORE,E123" # Closing bracket does not match indentation of opening bracket's line
-IGNORE="$IGNORE,E124" # Closing bracket missing visual indentation
-IGNORE="$IGNORE,E125" # Continuation line does not distinguish itself from next logical line
-IGNORE="$IGNORE,E126" # Continuation line over-indented for hanging indent
-IGNORE="$IGNORE,E127" # Continuation line over-indented for visual indent
-IGNORE="$IGNORE,E128" # Continuation line under-indented for visual indent
-IGNORE="$IGNORE,E502" # The backslash is redundant between brackets
-
-cmd=(
-    ${base}/hacking.py
-
-    --ignore="$IGNORE"
-
-    "${files[@]}"
-)
-
-echo -e "\nRunning 'cloudinit' pep8:"
-echo "${cmd[@]}"
-"${cmd[@]}"

=== removed file 'tools/run-pylint'
--- tools/run-pylint	2014-01-24 20:28:06 +0000
+++ tools/run-pylint	1970-01-01 00:00:00 +0000
@@ -1,26 +0,0 @@
-#!/bin/bash
-
-if [ $# -eq 0 ]; then
-   files=( bin/cloud-init $(find * -name "*.py" -type f) )
-else
-   files=( "$@" );
-fi 
-
-RC_FILE="pylintrc"
-if [ ! -f $RC_FILE ]; then
-    RC_FILE="../pylintrc"
-fi
-
-cmd=(
-    pylint
-    --rcfile=$RC_FILE
-    --disable=R
-    --disable=I
-    --dummy-variables-rgx="_"
-    "${files[@]}"
-)
-
-echo -e "\nRunning pylint:"
-echo "${cmd[@]}"
-"${cmd[@]}"
-

=== added file 'tox.ini'
--- tox.ini	1970-01-01 00:00:00 +0000
+++ tox.ini	2014-01-26 01:03:26 +0000
@@ -0,0 +1,49 @@
+[tox]
+minversion = 1.6
+skipsdist = True
+envlist = py26,py27,py33
+
+[testenv]
+usedevelop = True
+setenv = VIRTUAL_ENV={envdir}
+         LANG=en_US.UTF-8
+         LANGUAGE=en_US:en
+         LC_ALL=C
+deps = -r{toxinidir}/requirements.txt
+       -r{toxinidir}/test-requirements.txt
+commands = nosetests {posargs}
+install_command = pip install {opts} {packages}
+
+[testenv:venv]
+commands = {posargs}
+
+[testenv:flake8]
+commands = flake8 {posargs}
+
+[flake8]
+# See: http://pylint-messages.wikidot.com/all-codes
+# W0142: *args and **kwargs are fine.
+# W0511: TODOs in code comments are fine.
+# W0702: No exception type(s) specified
+# W0703: Catch "Exception"
+# C0103: Invalid name
+# C0111: Missing docstring
+# F841: local variable ABC is assigned to but never used
+# E121: Continuation line indentation is not a multiple of four
+# E123: Closing bracket does not match indentation of opening bracket's line
+# E124: Closing bracket missing visual indentation
+# E125: Continuation line does not distinguish itself from next logical line
+# E126: Continuation line over-indented for hanging indent
+# E127: Continuation line over-indented for visual indent
+# E128: Continuation line under-indented for visual indent
+# E502: The backslash is redundant between brackets
+# H201: no 'except:' at least use 'except Exception:'
+# H232: Python 3.x incompatible octal X should be written as Y
+# H233: Python 3.x incompatible use of print operator
+# H234: assertEquals is deprecated, use assertEqual
+# H301: one import per line
+# H302: import only modules.
+# H404: multi line docstring should start without a leading new line
+ignore = F841,E121,E123,E124,E125,E126,E127,E128,E502,H234,H201,H301,H404,H302,H232,H233,W0142,W0511,W0702,W0703,C0103,C0111
+exclude = .venv,.tox,dist,doc,*egg,.bzr,build,tools
+max-line-length = 79