refactor: Use communicate with a timeout for build and invoke integ t…

…ests (#1571)

appveyor.yml

@@ -61,18 +61,8 @@ for:
       - "pylint --rcfile .pylintrc samcli"
       # There are some functional tests that are currently broken due to not being updated with changed code or still running with node4.3 runtimes
       # We need to update those but this allows us to at least runs the ones we currently have working
-      - "pytest tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py"
+      - "pytest -n 4 tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py"
 
-      # Runs only in Linux
-      - sh: "pytest -vv tests/integration"
-      - sh: "/tmp/black --check setup.py tests samcli scripts"
-      - sh: "python scripts/check-isolated-needs-update.py"
-
-      # Smoke tests run in parallel - it runs on both Linux & Windows
-      # Presence of the RUN_SMOKE envvar will run the smoke tests
-      # Note: temporarily removing as with current dependencies we require syslog on windows
-      # which is not present on stdlib.
-      # - ps: "If ($env:RUN_SMOKE) {pytest -n 4 -vv tests/smoke}"
   - 
     matrix:
       only:
@@ -128,11 +118,11 @@ for:
       - "pylint --rcfile .pylintrc samcli"
       # There are some functional tests that are currently broken due to not being updated with changed code or still running with node4.3 runtimes
       # We need to update those but this allows us to at least runs the ones we currently have working
-      - "pytest tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py"
+      - "pytest -n 4 tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py"
 
       # Runs only in Linux
       - sh: "pytest -vv tests/integration"
-      - sh: "pytest -vv -n 4 tests/regression"
+      - sh: "pytest -vv tests/regression"
       - sh: "/tmp/black --check setup.py tests samcli scripts"
 
       # Set JAVA_HOME to java11
@@ -142,13 +132,3 @@ for:
       # Smoke tests run in parallel - it runs on both Linux & Windows
       # Presence of the RUN_SMOKE envvar will run the smoke tests
       - ps: "If ($env:RUN_SMOKE) {pytest -n 4 -vv tests/smoke}"
-
-  - 
-    matrix:
-      only:
-        - ONLY_SMOKE: 1
-
-    test_script:
-      # Smoke tests run in parallel
-      - sh: "venv/Scripts/activate"
-      - sh: "pytest -n 4 -vv tests/smoke"

tests/integration/buildcmd/test_build_cmd.py

@@ -1,10 +1,11 @@
 import sys
 import os
-import subprocess
 import logging
 from unittest import skipIf
 from pathlib import Path
 from parameterized import parameterized
+from subprocess import Popen, PIPE, TimeoutExpired
+
 import pytest
 
 from .build_integ_base import BuildIntegBase
@@ -13,6 +14,8 @@
 
 LOG = logging.getLogger(__name__)
 
+TIMEOUT = 300
+
 
 @skipIf(
     ((IS_WINDOWS and RUNNING_ON_CI) and not CI_OVERRIDE),
@@ -31,6 +34,7 @@ class TestBuildCommand_PythonFunctions(BuildIntegBase):
 
     FUNCTION_LOGICAL_ID = "Function"
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("python2.7", False),
@@ -48,8 +52,12 @@ def test_with_default_requirements(self, runtime, use_container):
         cmdlist = self.get_command_list(use_container=use_container, parameter_overrides=overrides)
 
         LOG.info("Running Command: {}", cmdlist)
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(
             self.default_build_dir, self.FUNCTION_LOGICAL_ID, self.EXPECTED_FILES_PROJECT_MANIFEST
@@ -98,15 +106,20 @@ def _get_python_version(self):
     "Skip build tests on windows when running in CI unless overridden",
 )
 class TestBuildCommand_ErrorCases(BuildIntegBase):
+    @pytest.mark.flaky(reruns=3)
     def test_unsupported_runtime(self):
         overrides = {"Runtime": "unsupportedpython", "CodeUri": "Python"}
         cmdlist = self.get_command_list(parameter_overrides=overrides)
 
         LOG.info("Running Command: {}", cmdlist)
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir, stdout=subprocess.PIPE)
-        process.wait()
-
-        process_stdout = b"".join(process.stdout.readlines()).strip().decode("utf-8")
+        process = Popen(cmdlist, cwd=self.working_dir, stdout=PIPE)
+        try:
+            stdout, _ = process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
+
+        process_stdout = stdout.strip().decode("utf-8")
         self.assertEqual(1, process.returncode)
 
         self.assertIn("Build Failed", process_stdout)
@@ -124,6 +137,7 @@ class TestBuildCommand_NodeFunctions(BuildIntegBase):
 
     FUNCTION_LOGICAL_ID = "Function"
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("nodejs6.10", False),
@@ -141,8 +155,12 @@ def test_with_default_package_json(self, runtime, use_container):
         cmdlist = self.get_command_list(use_container=use_container, parameter_overrides=overrides)
 
         LOG.info("Running Command: {}", cmdlist)
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(
             self.default_build_dir,
@@ -198,11 +216,11 @@ class TestBuildCommand_RubyFunctions(BuildIntegBase):
     FUNCTION_LOGICAL_ID = "Function"
 
     @pytest.mark.flaky(reruns=3)
-    @pytest.mark.timeout(timeout=300, method="thread")
     @parameterized.expand([("ruby2.5")])
     def test_building_ruby_in_container(self, runtime):
         self._test_with_default_gemfile(runtime, "use_container")
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand([("ruby2.5")])
     def test_building_ruby_in_process(self, runtime):
         self._test_with_default_gemfile(runtime, False)
@@ -212,8 +230,12 @@ def _test_with_default_gemfile(self, runtime, use_container):
         cmdlist = self.get_command_list(use_container=use_container, parameter_overrides=overrides)
 
         LOG.info("Running Command: {}".format(cmdlist))
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(
             self.default_build_dir,
@@ -283,7 +305,6 @@ class TestBuildCommand_Java(BuildIntegBase):
     UNIX_LINE_ENDING = b"\n"
 
     @pytest.mark.flaky(reruns=3)
-    @pytest.mark.timeout(timeout=300, method="thread")
     @parameterized.expand(
         [
             ("java8", USING_GRADLE_PATH, EXPECTED_FILES_PROJECT_MANIFEST_GRADLE),
@@ -301,6 +322,7 @@ class TestBuildCommand_Java(BuildIntegBase):
     def test_building_java_in_container(self, runtime, code_path, expected_files):
         self._test_with_building_java(runtime, code_path, expected_files, "use_container")
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("java8", USING_GRADLE_PATH, EXPECTED_FILES_PROJECT_MANIFEST_GRADLE),
@@ -313,6 +335,7 @@ def test_building_java_in_container(self, runtime, code_path, expected_files):
     def test_building_java8_in_process(self, runtime, code_path, expected_files):
         self._test_with_building_java(runtime, code_path, expected_files, False)
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("java11", USING_GRADLE_PATH, EXPECTED_FILES_PROJECT_MANIFEST_GRADLE),
@@ -333,8 +356,12 @@ def _test_with_building_java(self, runtime, code_path, expected_files, use_conta
             self._change_to_unix_line_ending(os.path.join(self.test_data_path, self.USING_GRADLEW_PATH, "gradlew"))
 
         LOG.info("Running Command: {}".format(cmdlist))
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(
             self.default_build_dir, self.FUNCTION_LOGICAL_ID, expected_files, self.EXPECTED_DEPENDENCIES
@@ -408,6 +435,7 @@ class TestBuildCommand_Dotnet_cli_package(BuildIntegBase):
         "HelloWorld.dll",
     }
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("dotnetcore2.0", "Dotnetcore2.0", None),
@@ -431,8 +459,12 @@ def test_with_dotnetcore(self, runtime, code_uri, mode):
         if mode:
             newenv["SAM_BUILD_MODE"] = mode
 
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir, env=newenv)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir, env=newenv)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(
             self.default_build_dir, self.FUNCTION_LOGICAL_ID, self.EXPECTED_FILES_PROJECT_MANIFEST
@@ -455,6 +487,7 @@ def test_with_dotnetcore(self, runtime, code_uri, mode):
 
         self.verify_docker_container_cleanedup(runtime)
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand([("dotnetcore2.0", "Dotnetcore2.0"), ("dotnetcore2.1", "Dotnetcore2.1")])
     def test_must_fail_with_container(self, runtime, code_uri):
         use_container = True
@@ -466,8 +499,12 @@ def test_must_fail_with_container(self, runtime, code_uri):
         cmdlist = self.get_command_list(use_container=use_container, parameter_overrides=overrides)
 
         LOG.info("Running Command: {}".format(cmdlist))
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         # Must error out, because container builds are not supported
         self.assertEqual(process.returncode, 1)
@@ -507,16 +544,18 @@ class TestBuildCommand_SingleFunctionBuilds(BuildIntegBase):
         "requirements.txt",
     }
 
+    @pytest.mark.flaky(reruns=3)
     def test_function_not_found(self):
         overrides = {"Runtime": "python3.7", "CodeUri": "Python", "Handler": "main.handler"}
         cmdlist = self.get_command_list(parameter_overrides=overrides, function_identifier="FunctionNotInTemplate")
 
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        stdout, stderr = process.communicate()
+        process = Popen(cmdlist, cwd=self.working_dir, stderr=PIPE)
+        _, stderr = process.communicate(timeout=TIMEOUT)
 
         self.assertEqual(process.returncode, 1)
         self.assertIn("FunctionNotInTemplate not found", str(stderr.decode("utf8")))
 
+    @pytest.mark.flaky(reruns=3)
     @parameterized.expand(
         [
             ("python3.7", False, "FunctionOne"),
@@ -532,8 +571,12 @@ def test_build_single_function(self, runtime, use_container, function_identifier
         )
 
         LOG.info("Running Command: {}", cmdlist)
-        process = subprocess.Popen(cmdlist, cwd=self.working_dir)
-        process.wait()
+        process = Popen(cmdlist, cwd=self.working_dir)
+        try:
+            process.communicate(timeout=TIMEOUT)
+        except TimeoutExpired:
+            process.kill()
+            raise
 
         self._verify_built_artifact(self.default_build_dir, function_identifier, self.EXPECTED_FILES_PROJECT_MANIFEST)