From 7d4afa516452333ff418617741906d46dba6c304 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 14 Feb 2019 10:17:53 +0100 Subject: [PATCH 1/4] Fix string conversion of 'SpawnedProcessTimeout' * Calling the `communicate` method returns a tuple of `_io.TextIOWrapper` objects if the subprocess has timed out. Therefore, using the `__str__` method of the `SpawnedProcessTimeout` exception raises a `TypeError`. * The `read` method shoud be used to retrieve the string of the `_io.TextIOWrapper` objects. --- reframe/utility/os_ext.py | 4 ++-- unittests/test_utility.py | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/reframe/utility/os_ext.py b/reframe/utility/os_ext.py index 05bd9377dd..eb34f23a22 100644 --- a/reframe/utility/os_ext.py +++ b/reframe/utility/os_ext.py @@ -25,8 +25,8 @@ def run_command(cmd, check=False, timeout=None, shell=False): except subprocess.TimeoutExpired as e: os.killpg(proc.pid, signal.SIGKILL) raise SpawnedProcessTimeout(e.cmd, - proc.stdout, - proc.stderr, timeout) from None + proc.stdout.read(), + proc.stderr.read(), timeout) from None completed = subprocess.CompletedProcess(args=shlex.split(cmd), returncode=proc.returncode, diff --git a/unittests/test_utility.py b/unittests/test_utility.py index e81853d9da..1c387aca5b 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -30,6 +30,15 @@ def test_command_timeout(self): except SpawnedProcessTimeout as e: self.assertEqual(e.timeout, 2) + def test_command_timeout_exception_string(self): + with self.assertRaises(SpawnedProcessTimeout) as e: + os_ext.run_command('sleep 3', timeout=2) + + try: + str(e.exception) + except: + self.fail('could not convert exception to string') + def test_command_async(self): from datetime import datetime From 5970e425a60d30bf1ab690d2d98e0c2391915a15 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 14 Feb 2019 10:27:33 +0100 Subject: [PATCH 2/4] Replace bare 'except' with a 'except TypeError' --- unittests/test_utility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 1c387aca5b..09c3f956e3 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -36,7 +36,7 @@ def test_command_timeout_exception_string(self): try: str(e.exception) - except: + except TypeError: self.fail('could not convert exception to string') def test_command_async(self): From 1c2a12c9c926edcf7b5953ea0ac68e80214d1b33 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Fri, 15 Feb 2019 16:27:55 +0100 Subject: [PATCH 3/4] Address PR comments --- unittests/test_utility.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 09c3f956e3..f0f6152b64 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -29,15 +29,8 @@ def test_command_timeout(self): self.fail('Expected timeout') except SpawnedProcessTimeout as e: self.assertEqual(e.timeout, 2) - - def test_command_timeout_exception_string(self): - with self.assertRaises(SpawnedProcessTimeout) as e: - os_ext.run_command('sleep 3', timeout=2) - - try: - str(e.exception) - except TypeError: - self.fail('could not convert exception to string') + # Try to get the string representation of the exception: see bug #658 + s = str(e) def test_command_async(self): from datetime import datetime From 273ae34d8c36370bbb6bf3b0d5a0561cbd23ee96 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 15 Feb 2019 16:38:19 +0100 Subject: [PATCH 4/4] Minor style changes --- unittests/test_utility.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index f0f6152b64..5d7baecacc 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -26,11 +26,12 @@ def test_command_error(self): def test_command_timeout(self): try: os_ext.run_command('sleep 3', timeout=2) - self.fail('Expected timeout') except SpawnedProcessTimeout as e: self.assertEqual(e.timeout, 2) - # Try to get the string representation of the exception: see bug #658 + # Try to get the string repr. of the exception: see bug #658 s = str(e) + else: + self.fail('expected timeout') def test_command_async(self): from datetime import datetime