Mesa (master): glsl/tests: Don't use tempfiles

[GitLab Mirror]

Module: Mesa
Branch: master
Commit: 4cc8c25d5615647203d0c861de1ecd193bd29fe9
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=4cc8c25d5615647203d0c861de1ecd193bd29fe9

Author: Michel Dänzer <mdaenzer at redhat.com>
Date:   Fri Mar  5 23:05:51 2021 +0100

glsl/tests: Don't use tempfiles

Use pipes for direct communication between child & parent process.

Using tempfiles sometimes resulted in hitting the meson timeout if there
was high filesystem pressure (I saw a single unlink system call take as
long as 4 seconds; attempts to re-use a single tempfile just shifted the
delays to truncate/close systems calls).

As a bonus, this gets the valgrind test actually working as intended.
It wasn't working because the tempfile passed to --log-file didn't exist
(due to the earlier os.close(fd)?).

v2:
* Wrap .read() in "with open()" (Dylan Baker)

Reviewed-by: Dylan Baker <dylan.c.baker at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9528>

---

 src/compiler/glsl/glcpp/tests/glcpp_test.py | 86 +++++++++++++----------------
 1 file changed, 37 insertions(+), 49 deletions(-)

diff --git a/src/compiler/glsl/glcpp/tests/glcpp_test.py b/src/compiler/glsl/glcpp/tests/glcpp_test.py
index d3fdc6b27d7..bbf348d0179 100644
--- a/src/compiler/glsl/glcpp/tests/glcpp_test.py
+++ b/src/compiler/glsl/glcpp/tests/glcpp_test.py
@@ -29,7 +29,6 @@ import io
 import os
 import subprocess
 import sys
-import tempfile
 
 # The meson version handles windows paths better, but if it's not available
 # fall back to shlex
@@ -51,34 +50,32 @@ def arg_parser():
     return parser.parse_args()
 
 
-def parse_test_file(filename, nl_format):
+def parse_test_file(contents, nl_format):
     """Check for any special arguments and return them as a list."""
     # Disable "universal newlines" mode; we can't directly use `nl_format` as
     # the `newline` argument, because the "bizarro" test uses something Python
     # considers invalid.
-    with io.open(filename, newline='') as f:
-        for l in f.read().split(nl_format):
+    for l in contents.decode('utf-8').split(nl_format):
             if 'glcpp-args:' in l:
                 return l.split('glcpp-args:')[1].strip().split()
     return []
 
 
-def test_output(glcpp, filename, expfile, nl_format='\n'):
+def test_output(glcpp, contents, expfile, nl_format='\n'):
     """Test that the output of glcpp is what we expect."""
-    extra_args = parse_test_file(filename, nl_format)
+    extra_args = parse_test_file(contents, nl_format)
 
-    with open(filename, 'rb') as f:
-        proc = subprocess.Popen(
-            glcpp + extra_args,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            stdin=subprocess.PIPE)
-        actual, _ = proc.communicate(f.read())
-        actual = actual.decode('utf-8')
-
-        if proc.returncode == 255:
-            print("Test returned general error, possibly missing linker")
-            sys.exit(77)
+    proc = subprocess.Popen(
+        glcpp + extra_args,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        stdin=subprocess.PIPE)
+    actual, _ = proc.communicate(contents)
+    actual = actual.decode('utf-8')
+
+    if proc.returncode == 255:
+        print("Test returned general error, possibly missing linker")
+        sys.exit(77)
 
     with open(expfile, 'r') as f:
         expected = f.read()
@@ -94,25 +91,19 @@ def test_output(glcpp, filename, expfile, nl_format='\n'):
 
 def _valgrind(glcpp, filename):
     """Run valgrind and report any warnings."""
-    extra_args = parse_test_file(filename, nl_format='\n')
-
-    try:
-        fd, tmpfile = tempfile.mkstemp()
-        os.close(fd)
-        with open(filename, 'rb') as f:
-            proc = subprocess.Popen(
-                ['valgrind', '--error-exitcode=31', '--log-file', tmpfile] + glcpp + extra_args,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT,
-                stdin=subprocess.PIPE)
-            proc.communicate(f.read())
-            if proc.returncode != 31:
-                return (True, [])
-        with open(tmpfile, 'rb') as f:
-            contents = f.read()
-        return (False, contents)
-    finally:
-        os.unlink(tmpfile)
+    with open(filename, 'rb') as f:
+        contents = f.read()
+    extra_args = parse_test_file(contents, nl_format='\n')
+
+    proc = subprocess.Popen(
+        ['valgrind', '--error-exitcode=31'] + glcpp + extra_args,
+        stdout=subprocess.DEVNULL,
+        stderr=subprocess.PIPE,
+        stdin=subprocess.PIPE)
+    _, errors = proc.communicate(contents)
+    if proc.returncode != 31:
+        return (True, [])
+    return (False, errors.decode('utf-8'))
 
 
 def test_unix(args):
@@ -129,7 +120,9 @@ def test_unix(args):
         total += 1
 
         testfile = os.path.join(args.testdir, filename)
-        valid, diff = test_output(args.glcpp, testfile, testfile + '.expected')
+        with open(testfile, 'rb') as f:
+            contents = f.read()
+        valid, diff = test_output(args.glcpp, contents, testfile + '.expected')
         if valid:
             passed += 1
             print('PASS')
@@ -157,17 +150,12 @@ def _replace_test(args, replace):
         print(   '{}:'.format(os.path.splitext(filename)[0]), end=' ')
         total += 1
         testfile = os.path.join(args.testdir, filename)
-        try:
-            fd, tmpfile = tempfile.mkstemp()
-            os.close(fd)
-            with io.open(testfile, 'rt') as f:
-                contents = f.read()
-            with io.open(tmpfile, 'wt') as f:
-                f.write(contents.replace('\n', replace))
-            valid, diff = test_output(
-                args.glcpp, tmpfile, testfile + '.expected', nl_format=replace)
-        finally:
-            os.unlink(tmpfile)
+
+        with open(testfile, 'rt') as f:
+            contents = f.read()
+        contents = contents.replace('\n', replace).encode('utf-8')
+        valid, diff = test_output(
+            args.glcpp, contents, testfile + '.expected', nl_format=replace)
 
         if valid:
             passed += 1