[URGENT] Fix the CI system (#6485)

It is about the exit codes that are returned from the various scripts in tools during tox execution.
Indeed, tox relies on the non-zero exit code from a given script to know that something failed during the execution.

Previously, theses scripts were in bash, and a bash script returns an exit code that is the higher code returned from any of the command executed by the script. So if any command return a non-zero (in particular pylint or pytest), then the script return also non-zero.

Now that these scripts are converted into python, pylint and pytest are executed via subprocess, that returns the exit code as variables. But if theses codes are not handled explicitly, the python script itself will return zero if no python exception occured. As a consequence currently, Certbot CI system is unable to detect any test error or lint error, because there is no exception in this case, only exit codes from the binaries executed.

This PR fixes that, by handling correctly the exit code from the most critical scripts, install_and_test.py and tox.cover.py, but also all the scripts that I converted into Python and that could be executed in the context of a shell (via tox or directly for instance).

tools/_venv_common.py

@@ -11,7 +11,7 @@ import sys
 
 def subprocess_with_print(command):
     print(command)
-    subprocess.call(command, shell=True)
+    return subprocess.call(command, shell=True)
 
 def get_venv_python(venv_path):
     python_linux = os.path.join(venv_path, 'bin/python')
@@ -35,17 +35,19 @@ 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())))
 
-    subprocess_with_print(' '.join([
+    exit_code = 0
+
+    exit_code = subprocess_with_print(' '.join([
         sys.executable, '-m', 'virtualenv', '--no-site-packages', '--setuptools',
-        venv_name, venv_args]))
+        venv_name, venv_args])) or exit_code
 
     python_executable = get_venv_python(venv_name)
 
-    subprocess_with_print(' '.join([
+    exit_code = 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')]
+    command = [python_executable, os.path.normpath('./tools/pip_install.py')] or exit_code
     command.extend(args)
-    subprocess_with_print(' '.join(command))
+    exit_code = subprocess_with_print(' '.join(command)) or exit_code
 
     if os.path.isdir(os.path.join(venv_name, 'bin')):
         # Linux/OSX specific
@@ -63,5 +65,9 @@ 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__':
-    main(os.environ.get('VENV_NAME', 'venv'), os.environ.get('VENV_ARGS', ''), sys.argv[1:])
+    sys.exit(main(os.environ.get('VENV_NAME', 'venv'),
+                  os.environ.get('VENV_ARGS', ''), 
+                  sys.argv[1:]))

tools/install_and_test.py

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

tools/pip_install.py

@@ -59,11 +59,14 @@ def merge_requirements(tools_path, test_constraints, all_constraints):
 
 def call_with_print(command, cwd=None):
     print(command)
-    subprocess.call(command, shell=True, cwd=cwd or os.getcwd())
+    return subprocess.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')
@@ -76,15 +79,17 @@ def main(args):
 
         merge_requirements(tools_path, test_constraints, all_constraints)
         if requirements:
-            call_with_print(' '.join([
+            exit_code = call_with_print(' '.join([
                 sys.executable, '-m', 'pip', 'install', '-q', '--constraint', all_constraints,
-                '--requirement', requirements]))
+                '--requirement', requirements])) or exit_code
 
         command = [sys.executable, '-m', 'pip', 'install', '-q', '--constraint', all_constraints]
         command.extend(args)
-        call_with_print(' '.join(command))
+        exit_code = call_with_print(' '.join(command)) or exit_code
     finally:
         shutil.rmtree(working_dir)
 
+    return exit_code
+
 if __name__ == '__main__':
-    main(sys.argv[1:])
+    sys.exit(main(sys.argv[1:]))

tools/pip_install_editable.py

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

tools/venv.py

@@ -5,6 +5,7 @@ from __future__ import absolute_import
 
 import os
 import subprocess
+import sys
 
 import _venv_common
 
@@ -52,7 +53,7 @@ def main():
 
     venv_args = get_venv_args()
 
-    _venv_common.main('venv', venv_args, REQUIREMENTS)
+    return _venv_common.main('venv', venv_args, REQUIREMENTS)
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())

tools/venv3.py

@@ -5,6 +5,7 @@ from __future__ import absolute_import
 
 import os
 import subprocess
+import sys
 
 import _venv_common
 
@@ -47,7 +48,7 @@ def get_venv_args():
 def main():
     venv_args = get_venv_args()
 
-    _venv_common.main('venv3', venv_args, REQUIREMENTS)
+    return _venv_common.main('venv3', venv_args, REQUIREMENTS)
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())