Skip to content

refactor: move all process.wait to process.communicate #1590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sriram-mv merged 3 commits into from
Nov 24, 2019
Merged

refactor: move all process.wait to process.communicate #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f83bcf
refactor: move all process.wait to process.communicate
jfuss Nov 23, 2019
90f8317
Fix layer integ tests
jfuss Nov 23, 2019
0da5cba
Strip before spliting string
jfuss Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 55 additions & 14 deletions tests/integration/deploy/test_deploy_command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import tempfile
import uuid
import time
from subprocess import Popen, PIPE
from subprocess import Popen, PIPE, TimeoutExpired
from unittest import skipIf

import boto3
Expand All @@ -18,6 +18,7 @@
# This is to restrict package tests to run outside of CI/CD and when the branch is not master.
SKIP_DEPLOY_TESTS = RUNNING_ON_CI and RUNNING_TEST_FOR_MASTER_ON_CI
CFN_SLEEP = 3
TIMEOUT = 300


@skipIf(SKIP_DEPLOY_TESTS, "Skip deploy tests in CI/CD only")
Expand All @@ -44,7 +45,11 @@ def test_package_and_deploy_no_s3_bucket_all_args(self, template_file):
)

package_process = Popen(package_command_list, stdout=PIPE)
package_process.wait()
try:
package_process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
package_process.kill()
raise

self.assertEqual(package_process.returncode, 0)

Expand All @@ -67,7 +72,11 @@ def test_package_and_deploy_no_s3_bucket_all_args(self, template_file):
)

deploy_process_no_execute = Popen(deploy_command_list_no_execute, stdout=PIPE)
deploy_process_no_execute.wait()
try:
deploy_process_no_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_no_execute.kill()
raise
self.assertEqual(deploy_process_no_execute.returncode, 0)

# Deploy the given stack with the changeset.
Expand All @@ -84,7 +93,11 @@ def test_package_and_deploy_no_s3_bucket_all_args(self, template_file):
)

deploy_process = Popen(deploy_command_list_execute, stdout=PIPE)
deploy_process.wait()
try:
deploy_process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process.kill()
raise
self.assertEqual(deploy_process.returncode, 0)

@parameterized.expand(["aws-serverless-function.yaml"])
Expand All @@ -111,7 +124,11 @@ def test_no_package_and_deploy_with_s3_bucket_all_args(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE)
deploy_process_execute.wait()
try:
deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
self.assertEqual(deploy_process_execute.returncode, 0)

@parameterized.expand(["aws-serverless-function.yaml"])
Expand All @@ -138,7 +155,7 @@ def test_no_package_and_deploy_with_s3_bucket_all_args_confirm_changeset(self, t
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE, stdin=PIPE)
deploy_process_execute.communicate("Y".encode())
deploy_process_execute.communicate("Y".encode(), timeout=TIMEOUT)
self.assertEqual(deploy_process_execute.returncode, 0)

@parameterized.expand(["aws-serverless-function.yaml"])
Expand All @@ -163,10 +180,14 @@ def test_deploy_without_s3_bucket(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE)
deploy_process_execute.wait()
try:
_, stderr = deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Error asking for s3 bucket
self.assertEqual(deploy_process_execute.returncode, 1)
stderr = b"".join(deploy_process_execute.stderr.readlines()).strip()
stderr = stderr.strip()
self.assertIn(
bytes(
f"S3 Bucket not specified, use --s3-bucket to specify a bucket name or run sam deploy --guided",
Expand Down Expand Up @@ -194,7 +215,11 @@ def test_deploy_without_stack_name(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE)
deploy_process_execute.wait()
try:
deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Error no stack name present
self.assertEqual(deploy_process_execute.returncode, 2)

Expand All @@ -219,7 +244,11 @@ def test_deploy_without_capabilities(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE)
deploy_process_execute.wait()
try:
deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Error capabilities not specified
self.assertEqual(deploy_process_execute.returncode, 1)

Expand All @@ -241,7 +270,11 @@ def test_deploy_without_template_file(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE)
deploy_process_execute.wait()
try:
deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Error template file not specified
self.assertEqual(deploy_process_execute.returncode, 1)

Expand All @@ -268,7 +301,11 @@ def test_deploy_with_s3_bucket_switch_region(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE)
deploy_process_execute.wait()
try:
deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Deploy should succeed
self.assertEqual(deploy_process_execute.returncode, 0)

Expand All @@ -290,10 +327,14 @@ def test_deploy_with_s3_bucket_switch_region(self, template_file):
)

deploy_process_execute = Popen(deploy_command_list, stdout=PIPE, stderr=PIPE)
deploy_process_execute.wait()
try:
_, stderr = deploy_process_execute.communicate(timeout=TIMEOUT)
except TimeoutExpired:
deploy_process_execute.kill()
raise
# Deploy should fail, asking for s3 bucket
self.assertEqual(deploy_process_execute.returncode, 1)
stderr = b"".join(deploy_process_execute.stderr.readlines()).strip()
stderr = stderr.strip()
self.assertIn(
bytes(
f"Error: Failed to create/update stack {stack_name} : "
Expand Down
44 changes: 33 additions & 11 deletions tests/integration/init/test_init_command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from unittest import TestCase
from subprocess import Popen
from subprocess import Popen, TimeoutExpired
import os
import tempfile

TIMEOUT = 300


class TestBasicInitCommand(TestCase):
def test_init_command_passes_and_dir_created(self):
Expand All @@ -24,9 +26,13 @@ def test_init_command_passes_and_dir_created(self):
temp,
]
)
return_code = process.wait()
try:
process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(return_code, 0)
self.assertEqual(process.returncode, 0)
self.assertTrue(os.path.isdir(temp + "/sam-app"))

def test_init_new_app_template(self):
Expand All @@ -48,9 +54,13 @@ def test_init_new_app_template(self):
temp,
]
)
return_code = process.wait()
try:
process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(return_code, 0)
self.assertEqual(process.returncode, 0)
self.assertTrue(os.path.isdir(temp + "/qs-scratch"))

def test_init_command_java_maven(self):
Expand All @@ -72,9 +82,13 @@ def test_init_command_java_maven(self):
temp,
]
)
return_code = process.wait()
try:
process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(return_code, 0)
self.assertEqual(process.returncode, 0)
self.assertTrue(os.path.isdir(temp + "/sam-app-maven"))

def test_init_command_java_gradle(self):
Expand All @@ -96,9 +110,13 @@ def test_init_command_java_gradle(self):
temp,
]
)
return_code = process.wait()
try:
process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(return_code, 0)
self.assertEqual(process.returncode, 0)
self.assertTrue(os.path.isdir(temp + "/sam-app-gradle"))

def test_init_command_with_extra_context_parameter(self):
Expand All @@ -122,9 +140,13 @@ def test_init_command_with_extra_context_parameter(self):
temp,
]
)
return_code = process.wait()
try:
process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(return_code, 0)
self.assertEqual(process.returncode, 0)
self.assertTrue(os.path.isdir(temp + "/sam-app-maven"))

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/local/generate_event/test_cli_integ.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
class Test_EventGeneration_Integ(TestCase):
def test_generate_event_substitution(self):
process = Popen([Test_EventGeneration_Integ._get_command(), "local", "generate-event", "s3", "put"])
return_code = process.wait()
self.assertEqual(return_code, 0)
process.communicate()
self.assertEqual(process.returncode, 0)

@staticmethod
def _get_command():
Expand Down
28 changes: 19 additions & 9 deletions tests/integration/local/invoke/runtimes/test_with_runtime_zips.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
import os
import tempfile

from subprocess import Popen, PIPE
from subprocess import Popen, PIPE, TimeoutExpired
from nose_parameterized import parameterized, param
import pytest

from tests.integration.local.invoke.invoke_integ_base import InvokeIntegBase
from pathlib import Path

TIMEOUT = 300


class TestWithDifferentLambdaRuntimeZips(InvokeIntegBase):
template = Path("runtimes", "template.yaml")
Expand All @@ -35,10 +37,14 @@ def test_runtime_zip(self, function_name):
)

process = Popen(command_list, stdout=PIPE)
return_code = process.wait()

self.assertEqual(return_code, 0)
process_stdout = b"".join(process.stdout.readlines()).strip()
try:
stdout, _ = process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(process.returncode, 0)
process_stdout = stdout.strip()
self.assertEqual(process_stdout.decode("utf-8"), '"Hello World"')

@pytest.mark.timeout(timeout=300, method="thread")
Expand All @@ -50,8 +56,12 @@ def test_custom_provided_runtime(self):
command_list = command_list + ["--skip-pull-image"]

process = Popen(command_list, stdout=PIPE)
return_code = process.wait()

self.assertEqual(return_code, 0)
process_stdout = b"".join(process.stdout.readlines()).strip()
try:
stdout, _ = process.communicate(timeout=TIMEOUT)
except TimeoutExpired:
process.kill()
raise

self.assertEqual(process.returncode, 0)
process_stdout = stdout.strip()
self.assertEqual(process_stdout.decode("utf-8"), '{"body":"hello 曰有冥 world 🐿","statusCode":200,"headers":{}}')
16 changes: 8 additions & 8 deletions tests/integration/local/invoke/test_integrations_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,10 +630,10 @@ def test_download_one_layer(self, function_logical_id):
process.kill()
raise

process_stdout = stdout.split(os.linesep)[-1:].strip()
process_stdout = stdout.decode("utf-8").strip().split(os.linesep)[-1]
expected_output = '"Layer1"'

self.assertEqual(process_stdout.decode("utf-8"), expected_output)
self.assertEqual(process_stdout, expected_output)

@parameterized.expand([("ChangedLayerVersionServerlessFunction"), ("ChangedLayerVersionLambdaFunction")])
def test_publish_changed_download_layer(self, function_logical_id):
Expand All @@ -656,10 +656,10 @@ def test_publish_changed_download_layer(self, function_logical_id):
process.kill()
raise

process_stdout = stdout.split(os.linesep).strip()
process_stdout = stdout.decode("utf-8").strip().split(os.linesep)[-1]
expected_output = '"Layer1"'

self.assertEqual(process_stdout.decode("utf-8"), expected_output)
self.assertEqual(process_stdout, expected_output)

self.layer_utils.upsert_layer(
layer_name=layer_name, ref_layer_name="ChangedLayerArn", layer_zip="changedlayer1.zip"
Expand All @@ -681,10 +681,10 @@ def test_publish_changed_download_layer(self, function_logical_id):
process.kill()
raise

process_stdout = stdout.split(os.linesep).strip()
process_stdout = stdout.decode("utf-8").strip().split(os.linesep)[-1]
expected_output = '"Changed_Layer_1"'

self.assertEqual(process_stdout.decode("utf-8"), expected_output)
self.assertEqual(process_stdout, expected_output)

@parameterized.expand([("TwoLayerVersionServerlessFunction"), ("TwoLayerVersionLambdaFunction")])
def test_download_two_layers(self, function_logical_id):
Expand All @@ -707,10 +707,10 @@ def test_download_two_layers(self, function_logical_id):

stdout = stdout

process_stdout = stdout.split(os.linesep).strip()
process_stdout = stdout.decode("utf-8").strip().split(os.linesep)[-1]
expected_output = '"Layer2"'

self.assertEqual(process_stdout.decode("utf-8"), expected_output)
self.assertEqual(process_stdout, expected_output)

def test_caching_two_layers(self):

Expand Down
Loading