use Python 'subprocess.Popen' as a context manager

Newer versions of Pylint suggest this.

diff --git a/rtest/rtest.py b/rtest/rtest.py
index 28b3836e5a95178513a7f135c3a8d2ec9b0cdea6..874b76fa57eee399cbbda2c5fb44d13b27761e78 100755 (executable)
--- a/rtest/rtest.py
+++ b/rtest/rtest.py
@@ -291,13 +291,10 @@ def doTest(test):
             file=sys.stderr)
       continue
 
-    result = subprocess.Popen(
-       testcmd,
-       universal_newlines=True,
-       stderr = subprocess.PIPE,
-    )
-    _, errout = result.communicate()
-    RVAL = result.returncode
+    with subprocess.Popen(testcmd, universal_newlines=True,
+                          stderr=subprocess.PIPE) as result:
+      _, errout = result.communicate()
+      RVAL = result.returncode
 
     if errout:
       print(errout)

diff --git a/rtest/test_regression.py b/rtest/test_regression.py
index 8813a93828b06b1ee1c46fc76a06c0df4c6f15a1..7672d4e7ac1bd73a8a74405b74e708c8636149e3 100644 (file)
--- a/rtest/test_regression.py
+++ b/rtest/test_regression.py
@@ -59,9 +59,9 @@ def test_regression_failure():
   """
 
   os.chdir(Path(__file__).resolve().parent)
-  result = subprocess.Popen([sys.executable, "rtest.py"],
-                            stderr=subprocess.PIPE, universal_newlines=True)
-  text = result.communicate()[1]
+  with subprocess.Popen([sys.executable, "rtest.py"], stderr=subprocess.PIPE,
+                        universal_newlines=True) as result:
+    text = result.communicate()[1]
   print(text)
   assert "Layout failures: 0" in text
 # FIXME: re-enable when all tests pass on all platforms
@@ -295,11 +295,11 @@ def test_793():
 
     # ask the VRML back end to handle a simple graph, using the above as the
     # current working directory
-    p = subprocess.Popen(["dot", "-Tvrml", "-o", os.devnull], cwd=t)
-    p.communicate("digraph { a -> b; }")
+    with subprocess.Popen(["dot", "-Tvrml", "-o", os.devnull], cwd=t) as p:
+      p.communicate("digraph { a -> b; }")
 
-  # Graphviz should not have caused a segfault
-  assert p.returncode != -signal.SIGSEGV, "Graphviz segfaulted"
+      # Graphviz should not have caused a segfault
+      assert p.returncode != -signal.SIGSEGV, "Graphviz segfaulted"
 
 def test_797():
   """
@@ -398,11 +398,11 @@ def test_1411():
   assert input.exists(), "unexpectedly missing test case"
 
   # process it with Graphviz (should fail)
-  p = subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull, input],
-    stderr=subprocess.PIPE, universal_newlines=True)
-  _, output = p.communicate()
+  with subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull, input],
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
+    _, output = p.communicate()
 
-  assert p.returncode != 0, "Graphviz accepted broken input"
+    assert p.returncode != 0, "Graphviz accepted broken input"
 
   assert "syntax error in line 17 near '\\'" in output, \
     'error message did not identify correct location'
@@ -433,11 +433,11 @@ def test_1444():
   assert input1.exists(), "unexpectedly missing test case"
 
   # ask Graphviz to process it
-  p = subprocess.Popen(["dot", "-Tsvg", input1], stdout=subprocess.PIPE,
-    stderr=subprocess.PIPE, universal_newlines=True)
-  stdout1, stderr = p.communicate()
+  with subprocess.Popen(["dot", "-Tsvg", input1], stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
+    stdout1, stderr = p.communicate()
 
-  assert p.returncode == 0, "failed to process a headport edge"
+    assert p.returncode == 0, "failed to process a headport edge"
 
   assert stderr.strip() == "", "emitted an error for a legal graph"
 
@@ -446,11 +446,11 @@ def test_1444():
   assert input2.exists(), "unexpectedly missing test case"
 
   # process it identically
-  p = subprocess.Popen(["dot", "-Tsvg", input2], stdout=subprocess.PIPE,
-    stderr=subprocess.PIPE, universal_newlines=True)
-  stdout2, stderr = p.communicate()
+  with subprocess.Popen(["dot", "-Tsvg", input2], stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
+    stdout2, stderr = p.communicate()
 
-  assert p.returncode == 0, "failed to process a headport edge"
+    assert p.returncode == 0, "failed to process a headport edge"
 
   assert stderr.strip() == "", "emitted an error for a legal graph"
 
@@ -464,14 +464,14 @@ def test_1449():
   """
 
   # start Graphviz
-  p = subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull],
-    stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-    universal_newlines=True)
+  with subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull],
+                        stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
 
-  # pass it some input that uses the SVG color scheme
-  _, stderr = p.communicate('graph g { colorscheme="svg"; }')
+    # pass it some input that uses the SVG color scheme
+    _, stderr = p.communicate('graph g { colorscheme="svg"; }')
 
-  assert p.returncode == 0, "Graphviz exited with non-zero status"
+    assert p.returncode == 0, "Graphviz exited with non-zero status"
 
   assert stderr.strip() == "", "SVG color scheme use caused warnings"
 
@@ -489,12 +489,13 @@ def test_1594():
   input = "1594.gvpr"
 
   # run GVPR with our (malformed) input program
-  p = subprocess.Popen(["gvpr", "-f", input], stdin=subprocess.PIPE,
-    cwd=os.path.join(os.path.dirname(__file__)),
-    stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
-  _, stderr = p.communicate()
+  with subprocess.Popen(["gvpr", "-f", input], stdin=subprocess.PIPE,
+                        cwd=os.path.join(os.path.dirname(__file__)),
+                        stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                        universal_newlines=True) as p:
+    _, stderr = p.communicate()
 
-  assert p.returncode != 0, "GVPR did not reject malformed program"
+    assert p.returncode != 0, "GVPR did not reject malformed program"
 
   assert "line 3:" in stderr, \
     "GVPR did not identify correct line of syntax error"
@@ -804,12 +805,13 @@ def test_html(src: Path):
   """
 
   # validate the file
-  p = subprocess.Popen(["xmllint", "--nonet", "--noout", "--html", "--valid",
-    src], stderr=subprocess.PIPE, universal_newlines=True)
-  _, stderr = p.communicate()
+  with subprocess.Popen(["xmllint", "--nonet", "--noout", "--html", "--valid",
+                         src], stderr=subprocess.PIPE,
+                        universal_newlines=True) as p:
+    _, stderr = p.communicate()
 
-  # expect it to succeed
-  assert p.returncode == 0
+    # expect it to succeed
+    assert p.returncode == 0
   assert stderr == ""
 
 def test_1855():
@@ -899,11 +901,11 @@ def test_1906():
   assert input.exists(), "unexpectedly missing test case"
 
   # use Circo to translate it to DOT
-  p = subprocess.Popen(["dot", "-Kcirco", "-Tgv", "-o", os.devnull, input],
-    stderr=subprocess.PIPE, universal_newlines=True)
-  _, stderr = p.communicate()
+  with subprocess.Popen(["dot", "-Kcirco", "-Tgv", "-o", os.devnull, input],
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
+    _, stderr = p.communicate()
 
-  assert p.returncode != 0, "graph that generates overflow was accepted"
+    assert p.returncode != 0, "graph that generates overflow was accepted"
 
   assert "area too large" in stderr, "missing/incorrect error message"
 
@@ -981,10 +983,11 @@ def test_1913():
     """
     run Dot with the given input and return its exit status and stderr
     """
-    p = subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull],
-      stdin=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
-    _, stderr = p.communicate(input)
-    return p.returncode, stderr
+    with subprocess.Popen(["dot", "-Tsvg", "-o", os.devnull],
+                          stdin=subprocess.PIPE, stderr=subprocess.PIPE,
+                          universal_newlines=True) as p:
+      _, stderr = p.communicate(input)
+      return p.returncode, stderr
 
   # Graphviz should accept all legal values for this attribute
   for align in ("left", "right", "center"):
@@ -1075,10 +1078,11 @@ def test_1971():
   # run edgepaint with an invalid option, `-rabbit`, that happens to have the
   # same first character as valid options
   args = ["edgepaint", "-rabbit"]
-  p = subprocess.Popen(args, stdin=subprocess.PIPE, universal_newlines=True)
-  p.communicate(input)
+  with subprocess.Popen(args, stdin=subprocess.PIPE,
+                        universal_newlines=True) as p:
+    p.communicate(input)
 
-  assert p.returncode != 0, "edgepaint incorrectly accepted '-rabbit'"
+    assert p.returncode != 0, "edgepaint incorrectly accepted '-rabbit'"
 
 @pytest.mark.xfail(strict=not is_ndebug_defined()) # FIXME
 def test_1990():
@@ -1128,11 +1132,12 @@ def test_2078():
           "}"
 
   # run it through Graphviz
-  p = subprocess.Popen(["dot", "-Tcanon", "-o", os.devnull],
-    stdin=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
-  _, stderr = p.communicate(input)
+  with subprocess.Popen(["dot", "-Tcanon", "-o", os.devnull],
+                        stdin=subprocess.PIPE, stderr=subprocess.PIPE,
+                        universal_newlines=True) as p:
+    _, stderr = p.communicate(input)
 
-  assert p.returncode != 0, "layout on subgraph was incorrectly accepted"
+    assert p.returncode != 0, "layout on subgraph was incorrectly accepted"
 
   assert "layout attribute is invalid except on the root graph" in stderr, \
     "expected warning not found"
@@ -1145,12 +1150,12 @@ def test_2078():
           "}"
 
   # ensure this one does not trigger warnings
-  p = subprocess.Popen(["dot", "-Tcanon", "-o", os.devnull],
-    stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-    universal_newlines=True)
-  stdout, stderr = p.communicate(input)
+  with subprocess.Popen(["dot", "-Tcanon", "-o", os.devnull],
+                        stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE, universal_newlines=True) as p:
+    stdout, stderr = p.communicate(input)
 
-  assert p.returncode == 0, "correct layout use was rejected"
+    assert p.returncode == 0, "correct layout use was rejected"
 
   assert stdout.strip() == "", "unexpected output"
   assert "layout attribute is invalid except on the root graph" not in stderr, \

diff --git a/rtest/test_tools.py b/rtest/test_tools.py
index 39219a96d1e2d5e9f8c4d46972559bc780c6fbfc..b3d431263440c9596abb28ef021d6390d67a32a4 100644 (file)
--- a/rtest/test_tools.py
+++ b/rtest/test_tools.py
@@ -83,11 +83,11 @@ def test_tools(tool):
   environ_copy.pop("DISPLAY", None)
 
   # Test usage
-  p = subprocess.Popen([tool, "-?"], env=environ_copy, stdin=subprocess.DEVNULL,
-                       stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
-                       universal_newlines=True)
-  output, _ = p.communicate()
-  ret = p.returncode
+  with subprocess.Popen([tool, "-?"], env=environ_copy,
+                        stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
+                        stderr=subprocess.STDOUT, universal_newlines=True) as p:
+    output, _ = p.communicate()
+    ret = p.returncode
 
   # FIXME: https://gitlab.com/graphviz/graphviz/-/issues/1934
   # cope with flaky failures, while also failing if this flakiness has been
@@ -98,16 +98,17 @@ def test_tools(tool):
     for _ in range(100):
       if has_pass and has_fail:
         break
-      p = subprocess.Popen([tool, "-?"], env=environ_copy,
-                           stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
-                           stderr=subprocess.STDOUT, universal_newlines=True)
-      out, _ = p.communicate()
-      if p.returncode == 0:
-        has_pass = True
-        ret = p.returncode
-        output = out
-      else:
-        has_fail = True
+      with subprocess.Popen([tool, "-?"], env=environ_copy,
+                            stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
+                            stderr=subprocess.STDOUT,
+                            universal_newlines=True) as p:
+        out, _ = p.communicate()
+        if p.returncode == 0:
+          has_pass = True
+          ret = p.returncode
+          output = out
+        else:
+          has_fail = True
     assert has_pass, "could not find passing execution"
     assert has_fail, "could not find failing execution (#1934 fixed?)"