[hawkmoth] Re: [PATCH 2/8] test: add python test runner

  • From: Jani Nikula <jani@xxxxxxxxxx>
  • To: Bruno Santos <brunomanuelsantos@xxxxxxxxxxxxxxxxxx>, hawkmoth@xxxxxxxxxxxxx
  • Date: Sun, 16 Dec 2018 21:21:21 +0200

On Sat, 15 Dec 2018, Bruno Santos <brunomanuelsantos@xxxxxxxxxxxxxxxxxx> wrote:

On 12/13/18 10:11 PM, Jani Nikula wrote:
Prepare for ditching cmdtest usage.
---
 test/test_hawkmoth.py | 35 ++++++++++++++++++++++++++++
 test/testenv.py       | 54 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100755 test/test_hawkmoth.py
 create mode 100644 test/testenv.py

diff --git a/test/test_hawkmoth.py b/test/test_hawkmoth.py
new file mode 100755
index 000000000000..1e5d0b598c81
--- /dev/null
+++ b/test/test_hawkmoth.py
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+# Copyright (c) 2018, Jani Nikula <jani@xxxxxxxxxx>
+# Licensed under the terms of BSD 2-Clause, see LICENSE for details.
+
+import os
+import unittest
+
+import testenv
+from hawkmoth import hawkmoth
+
+class ParserTest(unittest.TestCase):
+    def run_test(self, input_filename):
I think the standard practice is to use a leading '_' to denote private 
(however
fake) elements or double '__' for a form of name mangling (which is still a 
fake
kind of private guarantee).

Especially since this unittest module seems to do some introspection stuff to
figure out all the methods that have a specific name, maybe we shouldn't 
pollute
the namespace with random functions we use for our own stuff?

I say add a single '_' and call it a day.

Okay.


+        # sanity check
+        self.assertTrue(os.path.isfile(input_filename))
+
+        options = testenv.get_testcase_options(input_filename)
+        output = hawkmoth.parse_to_string(input_filename, False, **options)
+        expected = testenv.read_file(input_filename, ext='stdout')
+
+        self.assertEqual(expected, output)
+
+    def run_dir(self, path):
+        # sanity check
+        self.assertTrue(os.path.isdir(path))
+
+        with self.subTest(path=path):
+            for f in testenv.get_testcases(path):
+                with self.subTest(source=os.path.basename(f)):
+                    self.run_test(f)
+
+    def test_parser(self):
+        self.run_dir(testenv.testdir)
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/test/testenv.py b/test/testenv.py
new file mode 100644
index 000000000000..43998801a499
--- /dev/null
+++ b/test/testenv.py
@@ -0,0 +1,54 @@
+# Copyright (c) 2018, Jani Nikula <jani@xxxxxxxxxx>
+# Licensed under the terms of BSD 2-Clause, see LICENSE for details.
+
+import sys
+import os
+
+testext = '.c'
+testdir = os.path.dirname(os.path.abspath(__file__))
+rootdir = os.path.dirname(testdir)
+
+sys.path.append(rootdir)

sys.path.insert(0, rootdir)

This breaks if you have a different version or another module with the same 
name
on your path. Indeed, it breaks my setup and if it didn't, I would still be
running the unit tests with the system version which is not the desired 
effect.

Oops, thanks.


+
+def get_testcases(path):
+    return [os.path.join(path, f) for f in sorted(os.listdir(path))
+            if f.endswith(testext)]

s/[/(/
s/]/)/

You can use a generator here. I'm sure performance doesn't matter, but it's 
more
elegant.

If you do so, it will break one of the cases you use it on, and you'd need to 
do
list(get_testcases()), which is a bit ugly and defeats the purpose, or use
itertools.tee.

And while at it, this could be a bit more readable with yield, although 
opinions
may vary in that regard:

    for f in sorted(os.listdir(path)):
        if f.endswith(testext):
            yield os.path.join(path, f)


Up to you though.

I guess my only question is, won't the sorted() there build a list no
matter what? What does the generator gain us?

I think I lean towards the explict yield.

BR,
Jani.


+
+def get_testcase_options(testcase):
+    options_filename = modify_filename(testcase, ext='stdin')
+
+    # options are optional
+    options = {}
+    if os.path.isfile(options_filename):
+        with open(options_filename, 'r') as file:
+            for line in file.readlines():
+                line = line.strip()
+                # legacy
+                if line.startswith('--'):
+                    line = line[2:]
+
+                opt = line.split('=', 1)
+                options[opt[0]] = opt[1]
+
+    return options
+
+def modify_filename(filename, **kwargs):
+    ext = kwargs.get('ext')
+    if ext is not None:
+        base, extension = os.path.splitext(filename)
+        filename = base + '.' + ext
+
+    dirname = kwargs.get('dir')
+    if dirname is not None:
+        base = os.path.basename(filename)
+        filename = os.path.join(dirname, base)
+
+    return filename
+
+def read_file(filename, **kwargs):
+    filename = modify_filename(filename, **kwargs)
+
+    with open(filename, 'r') as file:
+        expected = file.read()
+
+    return expected


-- 
Bruno Santos

PGP KEY: 941052CD (pool.sks-keyservers.net)

Other related posts: