New expect_tests UI

expect_tests/test/pipeline_test.py

+import os
+import sys
+import unittest
+
+from expect_tests import pipeline
+from expect_tests.type_definitions import Test, MultiTest
+
+
+SCRIPT_DIR = os.path.abspath(os.path.dirname(__file__))
+DATA_DIR = os.path.join(SCRIPT_DIR, 'data')
+
+class ParsingAndConfigTest(unittest.TestCase):
+  def test_get_config(self):
+    """Testing that reading the config file works. Tests requires a specific
+    content for data/.expect_tests.cfg"""

[agable, 2014/11/12 03:15:10]

nit: First line of docstring should be a single line.

[pgervais, 2014/11/13 00:28:27]

Done.

+    path = os.path.join(SCRIPT_DIR, 'data')

[agable, 2014/11/12 03:15:10]

You already have DATA_DIR defined.

[pgervais, 2014/11/13 00:28:27]

Done.

+    black_list = pipeline.get_config(path)
+    self.assertEqual(black_list,
+                     set(['directory1', 'directory2', 'ignored']))
+
+  def test_parse_test_glob(self):
+    self.assertEqual(pipeline.parse_test_glob('a/b/c'),
+                     (os.path.abspath('a/b/c'), '*'))
+    self.assertEqual(pipeline.parse_test_glob('a/b/c:'),
+                     (os.path.abspath('a/b/c'), '*'))
+    self.assertEqual(pipeline.parse_test_glob('a/b/c:Test'),
+                     (os.path.abspath('a/b/c'), 'Test'))
+    self.assertEqual(pipeline.parse_test_glob('a/b/c:Test.Name'),
+                     (os.path.abspath('a/b/c'), 'Test.Name'))
+    self.assertRaises(ValueError, pipeline.parse_test_glob, 'a:b:c',)
+    self.assertRaises(ValueError, pipeline.parse_test_glob, 'a:b/c',)
+
+
+class PathManipulationTest(unittest.TestCase):
+  """Tests for all path-manipulating functions.
+
+  This set uses checked-out files in the present repository to avoid mocking
+  the I/O functions.
+  """
+
+  def test_get_python_root(self):

[agable, 2014/11/12 03:15:10]

I know this looks nice and compact, but this test tests four different
behaviors:
1) getting a dir with no init returns that dir
2) getting a package root returns its parent
3) getting a nested subpackage gets the parent of the root
4) getting a non-existent path raises an error
That really should be four different tests, so that different behaviors can be
modified independently. The same comment applies to the other tests below,
especially get_runtime_context. One test per behavior per function, not just one
test per function.

[iannucci, 2014/11/12 20:26:25]

I'm not sure I entirely agree with that.. 1 assert per test seems way too fine
grained, and you'll end up duplicating a lot to set up that 1 assert. However I
do agree that some of the longer tests could be broken up. I'd expect these to
look something like:

<common setup>  # probably a function call, maybe self.setUp()
<differential setup>
<assert block (1-5 asserts)>

IMO it's also great to do table-based tests, e.g.:

TESTS = [
  (<input>, <expected>)
]

for input, output in TESTS:
  # stuff
  # assert with context of (input, output)

The benefit being that you can clearly see the input and output side-by-side,
and it's clear to add another case, and easy to do so. But I'm not sure how well
it applies here.

[agable, 2014/11/12 23:45:11]

Please refer to go/bhnol (internal link, sorry). Test behaviors, not methods.
There can and often should be multiple asserts per test. But each of the four
commented blocks in test_get_runtime_context should obviously be a separate test
method.

[pgervais, 2014/11/13 00:28:27]

While I agree with what is stated in this document (separating behavior because
it helps with test maintenance) I think it is overkill in that case to write
separate functions, since testing each behavior is a single line each time. What
counts is that separated behaviors are tested in separate *places*, not
necessarily separate *functions*.

I'll add a comment here to tell that we're testing separate behaviors. And split
test_get_runtime_context into separate functions.

+    """This function uses the directory structure under data/"""
+    self.assertEqual(pipeline.get_python_root(DATA_DIR), DATA_DIR)
+    self.assertEqual(pipeline.get_python_root(
+      os.path.join(DATA_DIR, 'package1')), DATA_DIR)
+    self.assertEqual(pipeline.get_python_root(
+      os.path.join(DATA_DIR, 'package1', 'subpackage1_1')), DATA_DIR)
+    self.assertRaises(ValueError, pipeline.get_python_root,
+                      '____non-existing-path____')
+
+  def test_get_runtime_context(self):
+    """Test the get_runtime_context function"""
+    datadir = os.path.join(SCRIPT_DIR, 'data')

[agable, 2014/11/12 03:15:10]

DATA_DIR

[pgervais, 2014/11/13 00:28:27]

Done.

+
+    # Single directory case
+    test_globs = [datadir]
+    contexts = pipeline.get_runtime_contexts(test_globs)
+    self.assertEqual(len(contexts), 1)
+    self.assertEqual(contexts[0].cwd, datadir)
+    for testing_c in contexts[0].testing_contexts:
+      self.assertNotEqual(testing_c.package_name, 'ignored')
+
+    # Single package
+    test_globs = [os.path.join(datadir, 'package1')]
+    contexts = pipeline.get_runtime_contexts(test_globs)
+    self.assertEqual(len(contexts), 1)
+    self.assertEqual(contexts[0].cwd, datadir)
+    testing_c = contexts[0].testing_contexts
+    self.assertEqual(len(testing_c), 1)
+    self.assertEqual(testing_c[0].package_name, 'package1')
+
+    # Two packages in the same directory
+    test_globs = [os.path.join(datadir, 'package1'),
+                  os.path.join(datadir, 'package2')]
+    contexts = pipeline.get_runtime_contexts(test_globs)
+    self.assertEqual(len(contexts), 1)
+    self.assertEqual(contexts[0].cwd, datadir)
+    testing_c = contexts[0].testing_contexts
+    self.assertEqual(len(testing_c), 2)
+    package_names = set()
+    for testing_c in contexts[0].testing_contexts:
+      package_names.add(testing_c.package_name)
+
+    self.assertEqual(package_names, set(('package1', 'package2')))
+
+    # Packages inside directory
+    test_globs = [datadir, os.path.join(datadir, 'package1')]
+    contexts = pipeline.get_runtime_contexts(test_globs)
+    self.assertEqual(len(contexts), 1)
+    # 2 is the number of packages under datadir, not counting
+    # the ignored ones.
+    self.assertEqual(len(contexts[0].testing_contexts), 2)
+
+    test_globs = [datadir, os.path.join(datadir, 'package1'),
+                  os.path.join(datadir, 'package1', 'subpackage1_1')]
+    contexts = pipeline.get_runtime_contexts(test_globs)
+    self.assertEqual(len(contexts), 1)
+    self.assertEqual(len(contexts[0].testing_contexts), 2)
+

[agable, 2014/11/12 03:15:10]

Please have a test for what happens if you explicitly list an ignored dir (e.g.
test_globs = [os.path.join(datadir, 'other')]).

[pgervais, 2014/11/13 00:28:27]

I did not write such a test because the behavior is currently undefined. 
Shall we force processing even if the directory is ignored or not?

[agable, 2014/11/14 20:00:08]

Yeah, I asked for the test because the behavior is undefined and I wanted to
define it :) I don't know what should happen, but you should choose one and then
test it :)

+
+  def test_package_testing_context(self):
+    """Test the PackageTestingContext class"""
+    package_name = 'package1'
+    package1 = os.path.join(SCRIPT_DIR, 'data', package_name)
+    package2 = os.path.join(SCRIPT_DIR, 'data', 'package2')
+    other_package1 = os.path.join(SCRIPT_DIR, 'data', 'other', package_name)
+
+    # from_path
+    context = pipeline.PackageTestingContext.from_path(package1)
+    self.assertTrue(os.path.isabs(context.cwd))
+    self.assertTrue(len(context.package_name) > 0)
+    self.assertEqual(len(context.filters), 1)
+    self.assertEqual(context.filters[0][1], '*')
+
+    context = pipeline.PackageTestingContext.from_path(package1,
+                                                       filters='*')
+    self.assertTrue(os.path.isabs(context.cwd))
+    self.assertTrue(len(context.package_name) > 0)
+    self.assertEqual(len(context.filters), 1)
+    self.assertEqual(context.filters[0][1], '*')
+
+    context = pipeline.PackageTestingContext.from_path(package1,
+                                                       filters=('a*', 'b*'))
+    self.assertTrue(os.path.isabs(context.cwd))
+    self.assertTrue(len(context.package_name) > 0)
+    self.assertEqual(len(context.filters), 2)
+    self.assertEqual(context.filters[0][1], 'a*')
+    self.assertEqual(context.filters[1][1], 'b*')
+
+    # from_context_list
+    c1 = pipeline.PackageTestingContext.from_path(package1, filters='a')
+    c2 = pipeline.PackageTestingContext.from_path(package1, filters=('b','c'))
+    context = pipeline.PackageTestingContext.from_context_list((c1, c2))
+    self.assertEqual(len(context.filters), 3)
+    self.assertEqual(set(filt[1] for filt in context.filters),
+                     set(('a', 'b', 'c')))
+
+    c1 = pipeline.PackageTestingContext.from_path(package1, filters='a')
+    c2 = pipeline.PackageTestingContext.from_path(package2, filters=('b','c'))
+    self.assertRaises(AssertionError,
+                      pipeline.PackageTestingContext.from_context_list,
+                      (c1, c2))
+
+    # Same package name, different paths.
+    c1 = pipeline.PackageTestingContext.from_path(package1, filters='a')
+    c2 = pipeline.PackageTestingContext.from_path(other_package1,
+                                                  filters=('b','c'))
+    self.assertRaises(AssertionError,
+                      pipeline.PackageTestingContext.from_context_list,
+                      (c1, c2))
+
+    # Subpackage
+    subpackage_path = 'subpackage1_1'
+    subpackage1 = os.path.join(package1, subpackage_path)
+    c1 = pipeline.PackageTestingContext.from_path(subpackage1)
+    self.assertEqual(c1.package_name, 'package1')
+    self.assertEqual(c1.filters, [(subpackage_path, '*')])
+
+    c1 = pipeline.PackageTestingContext.from_path(subpackage1, filters='a*')
+    self.assertEqual(c1.package_name, 'package1')
+    self.assertEqual(c1.filters, [(subpackage_path, 'a*')])
+
+    for subpath, matcher in c1.itermatchers():
+      print matcher.pattern
+      self.assertIsNotNone(matcher.match('a'))
+      self.assertIsNotNone(matcher.match('ab'))
+      self.assertIsNone(matcher.match('ba'))
+      self.assertIsNone(matcher.match('b'))
+      self.assertEqual(subpath, subpackage_path)
+
+    # Test that a star is added to the filter.
+    c1 = pipeline.PackageTestingContext.from_path(subpackage1, filters='a')
+    self.assertEqual(c1.package_name, 'package1')
+    self.assertEqual(c1.filters, [(subpackage_path, 'a')])
+
+    for subpath, matcher in c1.itermatchers():
+      print matcher.pattern
+      self.assertIsNotNone(matcher.match('a'))
+      self.assertIsNotNone(matcher.match('ab'))
+      self.assertIsNone(matcher.match('ba'))
+      self.assertIsNone(matcher.match('b'))
+      self.assertEqual(subpath, subpackage_path)
+
+
+  def test_processing_context(self):
+    """Test the ProcessingContext class"""
+    package_name = 'package1'
+    package1 = os.path.join(SCRIPT_DIR, 'data', package_name)
+    subpackage1 = os.path.join(SCRIPT_DIR, 'data',
+                               package_name, 'subpackage1_1')
+    package2 = os.path.join(SCRIPT_DIR, 'data', 'package2')
+    other_package1 = os.path.join(SCRIPT_DIR, 'data', 'other', package_name)
+
+    c0 = pipeline.PackageTestingContext.from_path(package1)
+    c1 = pipeline.PackageTestingContext.from_path(package1, filters='a')
+    c2 = pipeline.PackageTestingContext.from_path(subpackage1, filters='d')
+    c3 = pipeline.PackageTestingContext.from_path(package2, filters=('b','c'))
+    c4 = pipeline.PackageTestingContext.from_path(other_package1)
+
+    # A processing context is a cwd + testing contexts.
+    # A testing context is cwd + one package name.
+    context = pipeline.ProcessingContext((c1, c2))
+    self.assertEqual(len(context.testing_contexts), 1)
+    self.assertEqual(set(filt[1]
+                         for filt in context.testing_contexts[0].filters),
+                     set(('a', 'd')))
+
+    context = pipeline.ProcessingContext((c0, c1, c2))
+    self.assertEqual(len(context.testing_contexts), 1)
+    self.assertEqual(set(filt[1]
+                         for filt in context.testing_contexts[0].filters),
+                     set(('*', 'a', 'd')))
+
+
+    context = pipeline.ProcessingContext((c1, c2, c3))
+    self.assertEqual(len(context.testing_contexts), 2)
+
+    # Fails because there are two different cwd.
+    self.assertRaises(ValueError, pipeline.ProcessingContext, (c1, c4))
+
+
+class TestListingTest(unittest.TestCase):
+  """Test functions related to listing tests."""
+  def test_walk_package(self):
+    """This function uses the directory structure under data/"""
+    modules = pipeline.walk_package('package1', DATA_DIR)
+    self.assertEqual(modules,
+                     ['package1.file1_test', 'package1.file2_test',
+                      'package1.subpackage1_1.file3_test',
+                      'package1.subpackage1_1.subpackage1_1_1.file4_test'])
+
+    modules = pipeline.walk_package('package1', DATA_DIR,
+                                    subpath='subpackage1_1')
+    self.assertEqual(modules,
+                     ['package1.subpackage1_1.file3_test',
+                      'package1.subpackage1_1.subpackage1_1_1.file4_test'])
+
+    self.assertRaises(ValueError, pipeline.walk_package,
+                      'package1', DATA_DIR, 'non-existing')
+
+  def test_get_test_gens_package(self):
+    def get_test_names(tests):
+      test_names = []
+
+      for gen in tests:
+        for test in gen():
+          if isinstance(test, MultiTest):
+            subtests = test.tests
+          else:
+            subtests = [test]
+
+          for subtest in subtests:
+            self.assertIsInstance(subtest, Test)
+            test_names.append(subtest.name)
+      return test_names
+
+    sys.path.insert(0, DATA_DIR)  # Ugh. But won't work otherwise.
+
+
+    # TODO(pgervais): add a MultiTest in a package under data/
+    package1 = os.path.join(DATA_DIR, 'package1')
+    testing_context = pipeline.PackageTestingContext.from_path(package1)
+    tests = pipeline.get_test_gens_package(testing_context)
+    self.assertEqual(
+      get_test_names(tests),
+      ['package1.file1_test.File1Test.test_trivial_1',
+       'package1.file2_test.File2Test.test_trivial_2',
+       'package1.subpackage1_1.file3_test.File3Test.test_trivial_3',
+       'package1.subpackage1_1.subpackage1_1_1.file4_test.' +
+           'File4Test.test_trivial_4'])
+
+    tests = pipeline.get_test_gens_package(
+      testing_context, subpath='subpackage1_1')
+    self.assertEqual(
+      get_test_names(tests),
+      ['package1.subpackage1_1.file3_test.File3Test.test_trivial_3',
+       'package1.subpackage1_1.subpackage1_1_1.file4_test.' +
+           'File4Test.test_trivial_4'])
+
+    tests = pipeline.get_test_gens_package(
+      testing_context, subpath='subpackage1_1/subpackage1_1_1')
+
+    self.assertEqual(
+      get_test_names(tests),
+      ['package1.subpackage1_1.subpackage1_1_1.file4_test.' +
+       'File4Test.test_trivial_4'])