From c152a9f7dcdb34a373d23838dce130811bbd1fa5 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Mon, 28 Nov 2016 13:15:42 -0800 Subject: [PATCH 1/2] Refactor noxfile Change-Id: Ica205635543f07cc1b8f5a619ab58576fed5da30 --- .coveragerc | 25 +- .travis.yml | 6 +- nox.py | 453 ++++++++---------- scripts/travis.sh | 2 +- testing/requirements-dev.txt | 69 --- .../{requirements-dev.in => requirements.txt} | 2 +- 6 files changed, 204 insertions(+), 353 deletions(-) delete mode 100644 testing/requirements-dev.txt rename testing/{requirements-dev.in => requirements.txt} (81%) diff --git a/.coveragerc b/.coveragerc index 5bd5bebff2d..2873cbd6afb 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,22 +1,11 @@ [run] -include = - appengine/* - bigquery/* - bigtable/* - blog/* - compute/* - datastore/* - dataproc/* - dns/* - error_reporting/* - language/* - logging/* - monitoring/* - pubsub/* - speech/* - storage/* - translate/* - vision/* +omit = + lib/* + env/* + */.nox/* + */conftest.py + */google_appengine/* + [report] exclude_lines = pragma: NO COVER diff --git a/.travis.yml b/.travis.yml index e7e066f668e..932fee9e9fc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,8 +23,8 @@ addons: - python3.5-dev install: - pip install --upgrade pip wheel virtualenv -- pip install nox-automation coverage +- pip install --upgrade nox-automation coverage +# Temporarily install this from source. +- pip install --upgrade git+https://github.com/dhermes/ci-diff-helper.git script: - ./scripts/travis.sh -after_script: -- coverage report diff --git a/nox.py b/nox.py index b0fb39aef85..980a1181b04 100644 --- a/nox.py +++ b/nox.py @@ -12,49 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. -""" -Noxfile used with nox-automation to run tests across all samples. - -Use nox -l to see all possible sessions. - -In general, you'll want to run: - - nox -s lint - # or - nox -s list -- /path/to/sample/dir - -And: - - nox -s tests -- /path/to/sample/dir - -""" +from __future__ import print_function import fnmatch import os -import subprocess import tempfile import nox -# Location of our common testing utilities. This isn't published to PyPI. -REPO_TOOLS_REQ =\ - 'git+https://github.com/GoogleCloudPlatform/python-repo-tools.git' - -# Arguments used for every invocation of py.test. -COMMON_PYTEST_ARGS = [ - '-x', '--no-success-flaky-report', '--cov', '--cov-config', - '.coveragerc', '--cov-append', '--cov-report='] - +try: + import ci_diff_helper +except ImportError: + ci_diff_helper = None -# Libraries that only work on Python 2.7 -PY27_ONLY_LIBRARIES = ['mysql-python'] -IGNORED_LIBRARIES = ['pylibmc'] - -# Whether we're running on Travis CI -ON_TRAVIS = os.environ.get('TRAVIS', False) +# +# Helpers and utility functions +# -def list_files(folder, pattern): +def _list_files(folder, pattern): """Lists all files below the given folder that match the pattern.""" for root, folders, files in os.walk(folder): for filename in files: @@ -62,8 +38,12 @@ def list_files(folder, pattern): yield os.path.join(root, filename) -def collect_sample_dirs(start_dir, blacklist=set(), suffix='_test.py'): - """Recursively collects a list of dirs that contain tests. +def _collect_dirs( + start_dir, + blacklist=set(['conftest.py', 'nox.py']), + suffix='_test.py'): + """Recursively collects a list of dirs that contain a file matching the + given suffix. This works by listing the contents of directories and finding directories that have `*_test.py` files. @@ -77,58 +57,39 @@ def collect_sample_dirs(start_dir, blacklist=set(), suffix='_test.py'): yield parent else: # Filter out dirs we don't want to recurse into - subdirs[:] = [s for s in subdirs - if s[0].isalpha() and - os.path.join(parent, s) not in blacklist] - - -def get_changed_files(): - """Uses travis environment variables to determine which files - have changed for this pull request / push.""" - # Debug info - print('TRAVIS_PULL_REQUEST: {}'.format( - os.environ.get('TRAVIS_PULL_REQUEST'))) - print('TRAVIS_COMMIT: {}'.format(os.environ.get('TRAVIS_COMMIT'))) - print('TRAVIS_BRANCH: {}'.format(os.environ.get('TRAVIS_BRANCH'))) - - pr = os.environ.get('TRAVIS_PULL_REQUEST') - if pr == 'false': - # This is not a pull request. - changed = subprocess.check_output( - ['git', 'show', '--pretty=format:', '--name-only', - os.environ.get('TRAVIS_COMMIT')]) - - elif pr is not None: - try: - changed = subprocess.check_output( - ['git', 'diff', '--name-only', - os.environ.get('TRAVIS_COMMIT'), - os.environ.get('TRAVIS_BRANCH')]) - - except subprocess.CalledProcessError: - # Fallback to git head. - git_head = subprocess.check_output( - ['git', 'rev-parse', 'HEAD']).strip() - - print('Falling back to HEAD: {}'.format(git_head)) - - changed = subprocess.check_output( - ['git', 'diff', '--name-only', - git_head, - os.environ.get('TRAVIS_BRANCH')]) - else: - changed = '' - print('Uh... where are we?') - return set([x for x in changed.split('\n') if x]) + subdirs[:] = [ + s for s in subdirs + if s[0].isalpha() and + os.path.join(parent, s) not in blacklist] -def filter_samples(sample_dirs, changed_files): +def _get_changed_files(): + """Returns a list of files changed for this pull request / push. + + If running on a public CI like Travis or Circle this is used to only + run tests/lint for changed files. + """ + if not ci_diff_helper: + return None + + try: + config = ci_diff_helper.get_config() + except OSError: # Not on CI. + return None + + changed_files = ci_diff_helper.get_changed_files('HEAD', config.base) + + changed_files = set([ + './{}'.format(filename) for filename in changed_files]) + + return changed_files + + +def _filter_samples(sample_dirs, changed_files): """Filers the list of sample directories to only include directories that - contain changed files.""" + contain files in the list of changed files.""" result = [] for sample_dir in sample_dirs: - if sample_dir.startswith('./'): - sample_dir = sample_dir[2:] for changed_file in changed_files: if changed_file.startswith(sample_dir): result.append(sample_dir) @@ -136,213 +97,183 @@ def filter_samples(sample_dirs, changed_files): return list(set(result)) -def setup_appengine(session): - """Installs the App Engine SDK.""" - # Install the app engine sdk and setup import paths. - if session.interpreter.startswith('python3'): - return +def _determine_local_import_names(start_dir): + """Determines all import names that should be considered "local". - gae_root = os.environ.get('GAE_ROOT', tempfile.gettempdir()) - session.env['GAE_SDK_PATH'] = os.path.join(gae_root, 'google_appengine') - session.run('gcprepotools', 'download-appengine-sdk', gae_root) + This is used when running the linter to insure that import order is + properly checked. + """ + return [ + basename + for basename, extension + in [os.path.splitext(path) for path + in os.listdir(start_dir)] + if extension == '.py' or os.path.isdir( + os.path.join(start_dir, basename)) + and basename not in ('__pycache__')] - # Create a lib directory to prevent the GAE vendor library from - # complaining. - if not os.path.exists('lib'): - os.makedirs('lib') +# +# App Engine specific helpers +# +_GAE_ROOT = os.environ.get('GAE_ROOT') +if _GAE_ROOT is None: + _GAE_ROOT = tempfile.mkdtemp() -def run_tests_in_sesssion( - session, interpreter, sample_directories, use_appengine=True, - skip_flaky=False): - """This is the main function for executing tests. - It: - 1. Install the common testing utilities. - 2. Installs the test requirements. - 3. Determines which pytest arguments to use. skip_flaky causes extra - arguments to be passed that will skip tests marked flaky. - 7. For each sample directory, it runs py.test. - """ - session.interpreter = interpreter - session.install(REPO_TOOLS_REQ) - session.install('-r', 'testing/requirements-dev.txt') - - if use_appengine: - setup_appengine(session) - - pytest_args = COMMON_PYTEST_ARGS[:] - - if skip_flaky or True: - pytest_args.append('-m not slow and not flaky') - - for sample in sample_directories: - # Ignore lib and env directories - ignore_args = [ - '--ignore', os.path.join(sample, 'lib'), - '--ignore', os.path.join(sample, 'env')] - - # output junit report. - junit_args = ['--junitxml', os.path.join(sample, 'junit.xml')] - - session.run( - 'py.test', sample, - *(pytest_args + ignore_args + junit_args), - success_codes=[0, 5]) # Treat no test collected as success. - - -@nox.parametrize('interpreter', ['python2.7', 'python3.5']) -def session_tests(session, interpreter): - """Runs tests for all non-gae standard samples.""" - # session.posargs is any leftover arguments from the command line, - # which allows users to run a particular test instead of all of them. - sample_directories = session.posargs - if not sample_directories: - sample_directories = collect_sample_dirs('.') - - run_tests_in_sesssion( - session, interpreter, sample_directories) - - -def session_gae(session): - """Runs test for GAE Standard samples.""" - sample_directories = collect_sample_dirs('appengine/standard') - run_tests_in_sesssion( - session, 'python2.7', sample_directories, use_appengine=True) - - -@nox.parametrize('subsession', ['gae', 'tests']) -def session_travis(session, subsession): - """On travis, just run with python3.5 and don't run slow or flaky tests.""" - if subsession == 'tests': - interpreter = 'python3.5' - sample_directories = collect_sample_dirs( - '.', set('./appengine/standard')) - elif subsession == 'gae': - interpreter = 'python2.7' - sample_directories = collect_sample_dirs('appengine/standard') - - changed_files = get_changed_files() - sample_directories = filter_samples( - sample_directories, changed_files) - - if not sample_directories: - print('No samples changed.') +def _setup_appengine_sdk(session): + """Installs the App Engine SDK, if needed.""" + session.env['GAE_SDK_PATH'] = os.path.join(_GAE_ROOT, 'google_appengine') + session.run('gcprepotools', 'download-appengine-sdk', _GAE_ROOT) + + +# +# Test sessions +# + + +PYTEST_COMMON_ARGS = [ + '-v', '--no-success-flaky-report', '--cov', + '--cov-config', os.path.abspath('.coveragerc'), '--cov-append', + '--cov-report', 'term', '--ignore', 'env', '--ignore', 'lib'] + +FLAKE8_COMMON_ARGS = [ + '--show-source', '--builtin', 'gettext', '--max-complexity', '15', + '--import-order-style', 'google', + '--exclude', '.nox,.cache,env,lib,generated_pb2', +] + +# Location of our common testing utilities. This isn't published to PyPI. +GCP_REPO_TOOLS_REQ =\ + 'git+https://github.com/GoogleCloudPlatform/python-repo-tools.git' + + +# Collect sample directories. +ALL_TESTED_SAMPLES = sorted(list(_collect_dirs('.'))) +ALL_SAMPLE_DIRECTORIES = sorted(list(_collect_dirs('.', suffix='.py'))) +GAE_STANDARD_SAMPLES = [ + sample for sample in ALL_TESTED_SAMPLES + if sample.startswith('./appengine/standard')] +NON_GAE_STANDARD_SAMPLES = sorted( + list(set(ALL_TESTED_SAMPLES) - set(GAE_STANDARD_SAMPLES))) + + +# Filter sample directories if on a CI like Travis or Circle to only run tests +# for changed samples. +CHANGED_FILES = _get_changed_files() + +if CHANGED_FILES is not None: + print('Filtering based on changed files.') + ALL_TESTED_SAMPLES = _filter_samples( + ALL_TESTED_SAMPLES, CHANGED_FILES) + ALL_SAMPLE_DIRECTORIES = _filter_samples( + ALL_SAMPLE_DIRECTORIES, CHANGED_FILES) + GAE_STANDARD_SAMPLES = _filter_samples( + GAE_STANDARD_SAMPLES, CHANGED_FILES) + NON_GAE_STANDARD_SAMPLES = _filter_samples( + NON_GAE_STANDARD_SAMPLES, CHANGED_FILES) + + +def _session_tests(session, sample): + """Runs py.test for a particular sample.""" + # This happens when there are no changed samples for the session. + if sample is None: + session.virtualenv = False return - print('Running tests on a subset of samples: ') - print('\n'.join(sample_directories)) + session.install('-r', 'testing/requirements.txt') + session.chdir(sample) + if os.path.exists(os.path.join(sample, 'requirements.txt')): + session.install('-r', 'requirements.txt') + session.run('pytest', *PYTEST_COMMON_ARGS) - run_tests_in_sesssion( - session, interpreter, sample_directories, skip_flaky=True) +@nox.parametrize('sample', GAE_STANDARD_SAMPLES) +def session_gae(session, sample): + """Runs py.test for an App Engine standard sample.""" + session.interpreter = 'python2.7' + session.install(GCP_REPO_TOOLS_REQ) + _setup_appengine_sdk(session) -def session_lint(session): - """Lints each sample.""" - sample_directories = session.posargs - if not sample_directories: - # The top-level dir isn't a sample dir - only its subdirs. - _, subdirs, _ = next(os.walk('.')) - sample_directories = ( - sample_dir for subdir in subdirs if not subdir.startswith('.') - for sample_dir in collect_sample_dirs( - subdir, suffix='.py', blacklist='conftest.py')) + # Create a lib directory if needed, otherwise the App Engine vendor library + # will complain. + if not os.path.isdir(os.path.join(sample, 'lib')): + os.mkdir(os.path.join(sample, 'lib')) - # On travis, only lint changed samples. - if ON_TRAVIS: - changed_files = get_changed_files() - sample_directories = filter_samples( - sample_directories, changed_files) + _session_tests(session, sample) - session.install('flake8', 'flake8-import-order') - for sample_directory in sample_directories: - # Determine local import names - local_names = [ - basename - for basename, extension - in [os.path.splitext(path) for path - in os.listdir(sample_directory)] - if extension == '.py' or os.path.isdir( - os.path.join(sample_directory, basename))] - - session.run( - 'flake8', - '--show-source', - '--builtin', 'gettext', - '--max-complexity', '15', - '--import-order-style', 'google', - '--exclude', '.nox,.cache,env,lib,generated_pb2', - '--application-import-names', ','.join(local_names), - sample_directory) - - -def session_reqcheck(session): - """Checks for out of date requirements.""" - session.install(REPO_TOOLS_REQ) +@nox.parametrize('sample', NON_GAE_STANDARD_SAMPLES) +def session_py27(session, sample): + """Runs py.test for a sample using Python 2.7""" + session.interpreter = 'python2.7' + _session_tests(session, sample) - if 'update' in session.posargs: - command = 'update-requirements' - else: - command = 'check-requirements' - reqfiles = list(list_files('.', 'requirements*.txt')) - reqfiles.append('testing/requirements-dev.in') +@nox.parametrize('sample', NON_GAE_STANDARD_SAMPLES) +def session_py35(session, sample): + """Runs py.test for a sample using Python 3.5""" + session.interpreter = 'python3.5' + _session_tests(session, sample) - for reqfile in reqfiles: - session.run('gcprepotools', command, reqfile) +@nox.parametrize('sample', ALL_SAMPLE_DIRECTORIES) +def session_lint(session, sample): + """Runs flake8 on the sample.""" + session.install('flake8', 'flake8-import-order') -def session_reqrollup(session): - """Rolls up all requirements files into requirements-dev.txt. + local_names = _determine_local_import_names(sample) + args = FLAKE8_COMMON_ARGS + [ + '--application-import-names', ','.join(local_names), + '.'] - This does not test for uniqueness. pip itself will validate that. - """ + session.chdir(sample) + session.run('flake8', *args) + + +# +# Utility sessions +# + + +def session_missing_tests(session): + """Lists all sample directories that do not have tests.""" session.virtualenv = False - requirements = set() - requirements_files = list(list_files('.', 'requirements*.txt')) - requirements_files.append('./testing/requirements-dev.in') + print('The following samples do not have tests:') + for sample in set(ALL_SAMPLE_DIRECTORIES) - set(ALL_TESTED_SAMPLES): + print('*', sample) - for filename in requirements_files: - if filename == './testing/requirements-dev.txt': - continue - with open(filename, 'r') as f: - lines = f.readlines() - requirements.update(lines) +SAMPLES_WITH_GENERATED_READMES = sorted( + list(_collect_dirs('.', suffix='.rst.in'))) - def mark_if_necessary(requirement): - """Adds environment markers to Python 2.7-only libraries.""" - for library in PY27_ONLY_LIBRARIES: - if requirement.startswith(library): - return '{}; python_version == \'2.7\'\n'.format( - requirement.strip()) - return requirement - def is_ignored(requirement): - """Ignores certain libraries.""" - for library in IGNORED_LIBRARIES: - if requirement.startswith(library): - return True +@nox.parametrize('sample', SAMPLES_WITH_GENERATED_READMES) +def session_readmegen(session, sample): + """(Re-)generates the readme for a sample.""" + session.install('jinja2', 'pyyaml') - requirements = [ - mark_if_necessary(requirement) for requirement in requirements] + if os.path.exists(os.path.join(sample, 'requirements.txt')): + session.install('-r', os.path.join(sample, 'requirements.txt')) - requirements = [ - requirement for requirement in requirements if not - is_ignored(requirement)] + in_file = os.path.join(sample, 'README.rst.in') + session.run('python', 'scripts/readme-gen/readme_gen.py', in_file) - with open('testing/requirements-dev.txt', 'w') as f: - f.write('# This file is generated by nox -s reqrollup. Do not edit.\n') - for requirement in sorted(requirements, key=lambda s: s.lower()): - if not requirement.startswith('#'): - f.write(requirement) +def session_check_requirements(session): + """Checks for out of date requirements and optionally updates them. -def session_readmegen(session): - session.install('-r', 'testing/requirements-dev.txt') + This is intentionally not parametric, as it's desired to never have two + samples with differing versions of dependencies. + """ + session.install(GCP_REPO_TOOLS_REQ) - in_files = list(list_files('.', 'README.rst.in')) + if 'update' in session.posargs: + command = 'update-requirements' + else: + command = 'check-requirements' - for in_file in in_files: - session.run('python', 'scripts/readme-gen/readme_gen.py', in_file) + reqfiles = list(_list_files('.', 'requirements*.txt')) + + for reqfile in reqfiles: + session.run('gcprepotools', command, reqfile) diff --git a/scripts/travis.sh b/scripts/travis.sh index bf7015cf4c1..dba50c68d53 100755 --- a/scripts/travis.sh +++ b/scripts/travis.sh @@ -6,7 +6,7 @@ if [[ $TRAVIS_SECURE_ENV_VARS == "true" ]]; then source ${TRAVIS_BUILD_DIR}/testing/test-env.sh; export GOOGLE_APPLICATION_CREDENTIALS=${TRAVIS_BUILD_DIR}/testing/service-account.json export GOOGLE_CLIENT_SECRETS=${TRAVIS_BUILD_DIR}/testing/client-secrets.json - nox --stop-on-first-error -s lint travis; + nox --stop-on-first-error -s lint gae py35; else # only run lint on external PRs echo 'External PR: only running lint.' diff --git a/testing/requirements-dev.txt b/testing/requirements-dev.txt deleted file mode 100644 index eef5bc369e4..00000000000 --- a/testing/requirements-dev.txt +++ /dev/null @@ -1,69 +0,0 @@ -# This file is generated by nox -s reqrollup. Do not edit. -beautifulsoup4==4.5.1 -coverage==4.2 -cryptography==1.5.3 -Django==1.10.3 -flake8==3.2.0 -flaky==3.3.0 -flask-cors==3.0.2 -Flask-SQLAlchemy==2.1 -flask==0.11.1 -Flask==0.11.1 -funcsigs==1.0.2 -functools32==3.2.3-2; python_version < "3" -google-api-python-client==1.5.5 -google-auth==0.3.1 -google-cloud-bigquery==0.21.0 -google-cloud-bigtable==0.21.0 -google-cloud-core==0.21.0 -google-cloud-datastore==0.21.0 -google-cloud-dns==0.21.0 -google-cloud-error-reporting==0.21.0 -google-cloud-happybase==0.20.0 -google-cloud-language==0.21.0 -google-cloud-logging==0.21.0 -google-cloud-pubsub==0.21.0 -google-cloud-speech==0.21.0 -google-cloud-storage==0.21.0 -google-cloud-translate==0.21.0 -google-cloud-vision==0.21.0 -google-cloud==0.21.0 -grpc-google-cloud-speech-v1beta1==0.11.1 -grpcio==1.0.1 -gunicorn==19.6.0 -httplib2==0.9.2 -kinto==4.3.4 -mailjet-rest==v1.2.2 -mock==2.0.0 -mysql-python==1.2.5; python_version == '2.7' -mysqlclient==1.3.9 -numpy==1.12.0b1 -oauth2client==4.0.0 -Pillow==3.4.2 -pyasn1-modules==0.0.8 -pyasn1==0.1.9 -PyAudio==0.2.9 -PyCrypto==2.6.1 -pyjwt==1.4.2 -PyMySQL==0.7.9 -pytest-cov==2.4.0 -pytest==3.0.4 -pyyaml==3.12 -redis==2.10.5 -requests-toolbelt==0.7.0 -requests==2.12.1 -requests[security]==2.12.1 -requests_toolbelt==0.7.0 -responses==0.5.1 -rsa==3.4.2 -scipy==0.18.1 -sendgrid==3.6.3 -simplejson==3.10.0 -six==1.10.0 -sleekxmpp==1.3.1 -SQLAlchemy==1.1.4 -twilio==6.3.dev0 -uritemplate==3.0.0 -webapp2==3.0.0b1 -WebTest==2.0.23 -wheel==0.30.0a0 diff --git a/testing/requirements-dev.in b/testing/requirements.txt similarity index 81% rename from testing/requirements-dev.in rename to testing/requirements.txt index da3f8472387..1fb7259e99c 100644 --- a/testing/requirements-dev.in +++ b/testing/requirements.txt @@ -3,7 +3,7 @@ coverage==4.2 flaky==3.3.0 funcsigs==1.0.2 mock==2.0.0 -mysql-python==1.2.5 +mysql-python==1.2.5; python_version < '3.0' PyCrypto==2.6.1 pytest-cov==2.4.0 pytest==3.0.4 From fd229cc95213ef462e2214f895bcd055bfa8c486 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Tue, 29 Nov 2016 10:07:35 -0800 Subject: [PATCH 2/2] Address review comments. Change-Id: I06a4ec7a542eb88ca367fd1936fa499aac4045ff --- nox.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nox.py b/nox.py index 980a1181b04..c9e0fc9abe1 100644 --- a/nox.py +++ b/nox.py @@ -103,11 +103,11 @@ def _determine_local_import_names(start_dir): This is used when running the linter to insure that import order is properly checked. """ + file_ext_pairs = [os.path.splitext(path) for path in os.listdir(start_dir)] return [ basename for basename, extension - in [os.path.splitext(path) for path - in os.listdir(start_dir)] + in file_ext_pairs if extension == '.py' or os.path.isdir( os.path.join(start_dir, basename)) and basename not in ('__pycache__')] @@ -241,7 +241,7 @@ def session_missing_tests(session): session.virtualenv = False print('The following samples do not have tests:') for sample in set(ALL_SAMPLE_DIRECTORIES) - set(ALL_TESTED_SAMPLES): - print('*', sample) + print('* {}'.format(sample)) SAMPLES_WITH_GENERATED_READMES = sorted(