Fail-fast in test/cover/lint scripts (#6487)

After #6485 and #6435, it appears that there is no good reason to not fail fast when test, cover or linting scripts are executed.

This PR ensures to fail fast by invoking commands throught subprocess.check_call instead of subprocess.call, and by removing the handling of non-zero exit code at the end of theses scripts.

As now coverage on Windows is executed with thresholds, I added specific thresholds for this platform. Because some portions of code that are done for Unix platform will not be executed on Windows.

Note that coverage reports from Travis and AppVeyor are accumulated on Codecov. So if a file is covered up to 50 % on Linux, and all other parts are covered on Windows, then coverage is 100 % for Codecov.

Note: that PR also fixes the ability of coverage tests to fail if thresholds are exceeded.

* Use check_call to fail fast in all scripts related to tests/lint/coverage/deploy

* Make specific coverage threshold for windows
This commit is contained in:
Adrien Ferrand 2018-11-14 22:57:40 +01:00 committed by Brad Warren
parent ad885afdb8
commit b3d2ac5161
8 changed files with 51 additions and 66 deletions

View file

@ -70,7 +70,7 @@ def validate_scripts_content(repo_path, temp_cwd):
# Compare file against the latest released version
latest_version = subprocess.check_output(
[sys.executable, 'fetch.py', '--latest-version'], cwd=temp_cwd)
subprocess.call(
subprocess.check_call(
[sys.executable, 'fetch.py', '--le-auto-script',
'v{0}'.format(latest_version.decode().strip())], cwd=temp_cwd)
if compare_files(
@ -95,7 +95,7 @@ def main():
os.path.normpath(os.path.join(repo_path, 'letsencrypt-auto-source/letsencrypt-auto')),
os.path.join(temp_cwd, 'original-lea')
)
subprocess.call([sys.executable, os.path.normpath(os.path.join(
subprocess.check_call([sys.executable, os.path.normpath(os.path.join(
repo_path, 'letsencrypt-auto-source/build.py'))])
shutil.copyfile(
os.path.normpath(os.path.join(repo_path, 'letsencrypt-auto-source/letsencrypt-auto')),

View file

@ -11,13 +11,13 @@ import sys
def subprocess_with_print(command):
print(command)
return subprocess.call(command, shell=True)
subprocess.check_call(command, shell=True)
def get_venv_python(venv_path):
python_linux = os.path.join(venv_path, 'bin/python')
python_windows = os.path.join(venv_path, 'Scripts\\python.exe')
if os.path.isfile(python_linux):
return python_linux
python_windows = os.path.join(venv_path, 'Scripts\\python.exe')
if os.path.isfile(python_windows):
return python_windows
@ -35,19 +35,17 @@ def main(venv_name, venv_args, args):
if os.path.isdir(venv_name):
os.rename(venv_name, '{0}.{1}.bak'.format(venv_name, int(time.time())))
exit_code = 0
exit_code = subprocess_with_print(' '.join([
subprocess_with_print(' '.join([
sys.executable, '-m', 'virtualenv', '--no-site-packages', '--setuptools',
venv_name, venv_args])) or exit_code
venv_name, venv_args]))
python_executable = get_venv_python(venv_name)
exit_code = subprocess_with_print(' '.join([
subprocess_with_print(' '.join([
python_executable, os.path.normpath('./letsencrypt-auto-source/pieces/pipstrap.py')]))
command = [python_executable, os.path.normpath('./tools/pip_install.py')] or exit_code
command = [python_executable, os.path.normpath('./tools/pip_install.py')]
command.extend(args)
exit_code = subprocess_with_print(' '.join(command)) or exit_code
subprocess_with_print(' '.join(command))
if os.path.isdir(os.path.join(venv_name, 'bin')):
# Linux/OSX specific
@ -65,9 +63,7 @@ def main(venv_name, venv_args, args):
else:
raise ValueError('Error, directory {0} is not a valid venv.'.format(venv_name))
return exit_code
if __name__ == '__main__':
sys.exit(main(os.environ.get('VENV_NAME', 'venv'),
os.environ.get('VENV_ARGS', ''),
sys.argv[1:]))
main(os.environ.get('VENV_NAME', 'venv'),
os.environ.get('VENV_ARGS', ''),
sys.argv[1:])

View file

@ -19,7 +19,7 @@ SKIP_PROJECTS_ON_WINDOWS = [
def call_with_print(command, cwd=None):
print(command)
return subprocess.call(command, shell=True, cwd=cwd or os.getcwd())
subprocess.check_call(command, shell=True, cwd=cwd or os.getcwd())
def main(args):
if os.environ.get('CERTBOT_NO_PIN') == '1':
@ -37,12 +37,10 @@ def main(args):
else:
new_args.append(arg)
exit_code = 0
for requirement in new_args:
current_command = command[:]
current_command.append(requirement)
exit_code = call_with_print(' '.join(current_command)) or exit_code
call_with_print(' '.join(current_command))
pkg = re.sub(r'\[\w+\]', '', requirement)
if pkg == '.':
@ -50,13 +48,11 @@ def main(args):
temp_cwd = tempfile.mkdtemp()
try:
exit_code = call_with_print(' '.join([
call_with_print(' '.join([
sys.executable, '-m', 'pytest', '--numprocesses', 'auto',
'--quiet', '--pyargs', pkg.replace('-', '_')]), cwd=temp_cwd) or exit_code
'--quiet', '--pyargs', pkg.replace('-', '_')]), cwd=temp_cwd)
finally:
shutil.rmtree(temp_cwd)
return exit_code
if __name__ == '__main__':
sys.exit(main(sys.argv[1:]))
main(sys.argv[1:])

View file

@ -59,14 +59,12 @@ def merge_requirements(tools_path, test_constraints, all_constraints):
def call_with_print(command, cwd=None):
print(command)
return subprocess.call(command, shell=True, cwd=cwd or os.getcwd())
subprocess.check_call(command, shell=True, cwd=cwd or os.getcwd())
def main(args):
tools_path = find_tools_path()
working_dir = tempfile.mkdtemp()
exit_code = 0
try:
test_constraints = os.path.join(working_dir, 'test_constraints.txt')
all_constraints = os.path.join(working_dir, 'all_constraints.txt')
@ -79,17 +77,15 @@ def main(args):
merge_requirements(tools_path, test_constraints, all_constraints)
if requirements:
exit_code = call_with_print(' '.join([
call_with_print(' '.join([
sys.executable, '-m', 'pip', 'install', '-q', '--constraint', all_constraints,
'--requirement', requirements])) or exit_code
'--requirement', requirements]))
command = [sys.executable, '-m', 'pip', 'install', '-q', '--constraint', all_constraints]
command.extend(args)
exit_code = call_with_print(' '.join(command)) or exit_code
call_with_print(' '.join(command))
finally:
shutil.rmtree(working_dir)
return exit_code
if __name__ == '__main__':
sys.exit(main(sys.argv[1:]))
main(sys.argv[1:])

View file

@ -14,7 +14,7 @@ def main(args):
new_args.append('-e')
new_args.append(arg)
return pip_install.main(new_args)
pip_install.main(new_args)
if __name__ == '__main__':
sys.exit(main(sys.argv[1:]))
main(sys.argv[1:])

View file

@ -53,7 +53,7 @@ def main():
venv_args = get_venv_args()
return _venv_common.main('venv', venv_args, REQUIREMENTS)
_venv_common.main('venv', venv_args, REQUIREMENTS)
if __name__ == '__main__':
sys.exit(main())
main()

View file

@ -48,7 +48,7 @@ def get_venv_args():
def main():
venv_args = get_venv_args()
return _venv_common.main('venv3', venv_args, REQUIREMENTS)
_venv_common.main('venv3', venv_args, REQUIREMENTS)
if __name__ == '__main__':
sys.exit(main())
main()

View file

@ -12,36 +12,33 @@ DEFAULT_PACKAGES = [
'certbot_dns_sakuracloud', 'certbot_nginx', 'certbot_postfix', 'letshelp_certbot']
COVER_THRESHOLDS = {
'certbot': 98,
'acme': 100,
'certbot_apache': 100,
'certbot_dns_cloudflare': 98,
'certbot_dns_cloudxns': 99,
'certbot_dns_digitalocean': 98,
'certbot_dns_dnsimple': 98,
'certbot_dns_dnsmadeeasy': 99,
'certbot_dns_gehirn': 97,
'certbot_dns_google': 99,
'certbot_dns_linode': 98,
'certbot_dns_luadns': 98,
'certbot_dns_nsone': 99,
'certbot_dns_ovh': 97,
'certbot_dns_rfc2136': 99,
'certbot_dns_route53': 92,
'certbot_dns_sakuracloud': 97,
'certbot_nginx': 97,
'certbot_postfix': 100,
'letshelp_certbot': 100
'certbot': {'linux': 98, 'windows': 94},
'acme': {'linux': 100, 'windows': 99},
'certbot_apache': {'linux': 100, 'windows': 100},
'certbot_dns_cloudflare': {'linux': 98, 'windows': 98},
'certbot_dns_cloudxns': {'linux': 99, 'windows': 99},
'certbot_dns_digitalocean': {'linux': 98, 'windows': 98},
'certbot_dns_dnsimple': {'linux': 98, 'windows': 98},
'certbot_dns_dnsmadeeasy': {'linux': 99, 'windows': 99},
'certbot_dns_gehirn': {'linux': 97, 'windows': 97},
'certbot_dns_google': {'linux': 99, 'windows': 99},
'certbot_dns_linode': {'linux': 98, 'windows': 98},
'certbot_dns_luadns': {'linux': 98, 'windows': 98},
'certbot_dns_nsone': {'linux': 99, 'windows': 99},
'certbot_dns_ovh': {'linux': 97, 'windows': 97},
'certbot_dns_rfc2136': {'linux': 99, 'windows': 99},
'certbot_dns_route53': {'linux': 92, 'windows': 92},
'certbot_dns_sakuracloud': {'linux': 97, 'windows': 97},
'certbot_nginx': {'linux': 97, 'windows': 97},
'certbot_postfix': {'linux': 100, 'windows': 100},
'letshelp_certbot': {'linux': 100, 'windows': 100}
}
SKIP_PROJECTS_ON_WINDOWS = [
'certbot-apache', 'certbot-nginx', 'certbot-postfix', 'letshelp-certbot']
def cover(package):
threshold = COVER_THRESHOLDS.get(package)
if not threshold:
raise ValueError('Unrecognized package: {0}'.format(package))
threshold = COVER_THRESHOLDS.get(package)['windows' if os.name == 'nt' else 'linux']
pkg_dir = package.replace('_', '-')
@ -51,10 +48,10 @@ def cover(package):
.format(pkg_dir)))
return
subprocess.call([
subprocess.check_call([
sys.executable, '-m', 'pytest', '--cov', pkg_dir, '--cov-append', '--cov-report=',
'--numprocesses', 'auto', '--pyargs', package])
subprocess.call([
subprocess.check_call([
sys.executable, '-m', 'coverage', 'report', '--fail-under', str(threshold), '--include',
'{0}/*'.format(pkg_dir), '--show-missing'])