Add unit tests for overrides in multi-repo workflow

unittests/override_test.py

Index: unittests/override_test.py
diff --git a/unittests/override_test.py b/unittests/override_test.py
new file mode 100755
index 0000000000000000000000000000000000000000..b0771f1f70aab2288ed1c30bdb6068a5d8260cdf
--- /dev/null
+++ b/unittests/override_test.py
@@ -0,0 +1,85 @@
+#!/usr/bin/env python
+# Copyright 2017 The LUCI Authors. All rights reserved.
+# Use of this source code is governed under the Apache License, Version 2.0
+# that can be found in the LICENSE file.
+
+import subprocess
+import sys
+import unittest
+
+import repo_test_util
+
+
+class TestOverride(repo_test_util.RepoTest):
+  def test_simple(self):
+    repos = self.repo_setup({
+        'a': [],
+        'a1': [],
+        'b': ['a'],
+    })
+
+    a_c1 = self.update_recipe_module(repos['a'], 'a_module', {'foo': ['bar']})
+    b_c1_rev = self.update_recipes_cfg(
+        'b', self.updated_package_spec_pb(repos['b'], 'a', a_c1['revision']))
+    b_c2 = self.update_recipe(
+        repos['b'], 'b_recipe', ['a/a_module'], [('a_module', 'foo')])
+
+    # Training the recipes should work.
+    self.train_recipes(repos['b'])
+
+    # Using a no-op override should also work.
+    self.train_recipes(repos['b'], overrides=[('a', repos['a']['root'])])
+
+    # Using an override pointing to empty repo should fail.
+    with self.assertRaises(subprocess.CalledProcessError) as cm:
+      self.train_recipes(repos['b'], overrides=[('a', repos['a1']['root'])])
+    self.assertIn(
+        'Exception: While generating results for \'b_recipe\': ImportError: '
+        'No module named a_module',
+        cm.exception.output)

[iannucci, 2017/01/31 18:22:54]

hm... it's a bit weird to use cm out of scope like this, but I guess it's
necessary?

[Paweł Hajdan Jr., 2017/01/31 18:30:54]

This is straight from python docs,
https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises .

The variable remains valid, and the scope is intended to be a block where we
catch the exception, not where we inspect it. Seems reasonable to me.

+
+  def test_dependency_conflict(self):
+    repos = self.repo_setup({
+        'a': [],
+        'b': ['a'],
+        'b1': ['a'],
+        'c': ['a', 'b'],
+    })
+
+    a_c1 = self.update_recipe_module(repos['a'], 'a_module', {'foo': ['bar']})
+    b_c1_rev = self.update_recipes_cfg(
+        'b', self.updated_package_spec_pb(repos['b'], 'a', a_c1['revision']))
+    b1_c1_rev = self.update_recipes_cfg(
+        'b1', self.updated_package_spec_pb(repos['b1'], 'a', a_c1['revision']))
+
+    a_c2 = self.update_recipe_module(repos['a'], 'a_module', {'foo': ['baz']})
+    b_c2_rev = self.update_recipes_cfg(
+        'b', self.updated_package_spec_pb(repos['b'], 'a', a_c2['revision']))
+
+    c_spec = self.get_package_spec(repos['c'])
+    c_spec.deps['a'].revision = a_c2['revision']
+    c_spec.deps['b'].revision = b_c2_rev
+    c_c1 = self.update_recipes_cfg('c', c_spec.dump())
+
+    c_c2 = self.update_recipe(
+        repos['c'], 'c_recipe', ['a/a_module'], [('a_module', 'foo')])
+
+    # Training the recipes should work.
+    self.train_recipes(repos['c'])

[iannucci, 2017/01/31 18:22:54]

should there be a way to assert the output of training? e.g. we should be able
to see that c calls a_module.foo, which runs step 'baz' here, correct?

[Paweł Hajdan Jr., 2017/01/31 18:30:54]

That's an internal part of update_recipe call, which itself calls train_recipes
anyway.

I put it again in the test to further emphasize failure when overriding.

In theory we could make this even more elaborate as you said and inspect the
contents of the new expectations, but I'm not sure if that's worth it. I'd
prefer each thing to be tested orthogonally (i.e. simulation tests tested
somewhere else from here), and for that reason deliberately avoided using
autoroller here and instead put the right revisions manually.

+
+    # Trying to override just the b repo should fail (conflicting revisions
+    # of the a repo).
+    with self.assertRaises(subprocess.CalledProcessError) as cm:
+      self.train_recipes(repos['c'], overrides=[('b', repos['b1']['root'])])
+    # TODO(phajdan.jr): assert on full message after making exception printable.
+    self.assertIn('InconsistentDependencyGraphError', cm.exception.output)
+
+    # The solution is to also override the a repo to make it clear what revision
+    # should be used.
+    self.train_recipes(
+        repos['c'],
+        overrides=[('b', repos['b1']['root']), ('a', repos['a']['root'])])
+
+
+if __name__ == '__main__':
+  sys.exit(unittest.main())