Fix #108: Don't add reftests till we compute the graph (#109)

* Fix #108: Don't add reftests till we compute the graph

manifest/manifest.py

@@ -46,6 +46,8 @@ def _included_items(self, include_types=None):
                 if item_type == "reftest":
                     for path, items in self.local_changes.iterdeletedreftests():
                         paths[path] -= items
+                        if len(paths[path]) == 0:
+                            del paths[path]
 
             yield item_type, paths
 
@@ -56,13 +58,10 @@ def add(self, item):
         if item is None:
             return
 
-        is_reference = False
         if isinstance(item, RefTest):
             self.reftest_nodes[item.path].add(item)
             self.reftest_nodes_by_url[item.url] = item
-            is_reference = item.is_reference
-
-        if not is_reference:
+        else:
             self._add(item)
 
         item.manifest = self
@@ -281,6 +280,7 @@ def from_json(cls, tests_root, obj):
                                                     tests_root,
                                                     obj["local_changes"],
                                                     source_files=source_files)
+        self.update_reftests()
         return self
 
 
@@ -297,13 +297,10 @@ def add(self, item):
         if item is None:
             return
 
-        is_reference = False
         if isinstance(item, RefTest):
             self.reftest_nodes[item.path].add(item)
             self.reftest_nodes_by_url[item.url] = item
-            is_reference = item.is_reference
-
-        if not is_reference:
+        else:
             self._add(item)
 
         item.manifest = self.manifest

manifest/tests/test_manifest.py

@@ -6,6 +6,7 @@ def test_local_reftest_add():
     s = sourcefile.SourceFile("/", "test", "/")
     test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
     m.local_changes.add(test)
+    m.update_reftests()
     assert list(m) == [(test.path, {test})]
 
 
@@ -15,6 +16,7 @@ def test_local_reftest_delete_path():
     test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
     m.add(test)
     m.local_changes.add_deleted(test.path)
+    m.update_reftests()
     assert list(m) == []
 
 
@@ -23,18 +25,19 @@ def test_local_reftest_adjusted():
     s = sourcefile.SourceFile("/", "test", "/")
     test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
     m.add(test)
-
-    assert list(m) == [(test.path, {test})]
+    m.update_reftests()
 
     assert m.compute_reftests({test.path: {test}}) == {test}
 
-    test_1 = manifestitem.RefTest(s, "/test-1", [("/test", "==")])
+    assert list(m) == [(test.path, {test})]
+
+    s_1 = sourcefile.SourceFile("/", "test-1", "/")
+    test_1 = manifestitem.RefTest(s_1, "/test-1", [("/test", "==")])
     m.local_changes.add(test_1)
+    m.update_reftests()
 
     assert m.compute_reftests({test.path: {test}, test_1.path: {test_1}}) == {test_1}
 
-    m.local_changes._deleted_reftests[test.path] = {test}
-
     assert list(m) == [(test_1.path, {test_1})]
 
 
@@ -43,13 +46,35 @@ def test_manifest_to_json():
     s = sourcefile.SourceFile("/", "test", "/")
     test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
     m.add(test)
-    test_1 = manifestitem.RefTest(s, "/test-1", [("/test", "==")])
+    s_1 = sourcefile.SourceFile("/", "test-1", "/")
+    test_1 = manifestitem.RefTest(s_1, "/test-1", [("/test", "==")])
     m.local_changes.add(test_1)
-    m.local_changes._deleted_reftests[test.path] = {test}
+    m.local_changes.add_deleted(test.path)
+    m.update_reftests()
 
     json_str = m.to_json()
     loaded = manifest.Manifest.from_json("/", json_str)
 
     assert list(loaded) == list(m)
 
     assert loaded.to_json() == json_str
+
+
+def test_reftest_computation_chain():
+    m = manifest.Manifest()
+
+    s1 = sourcefile.SourceFile("/", "test1", "/")
+    s2 = sourcefile.SourceFile("/", "test2", "/")
+
+    test1 = manifestitem.RefTest(s1, "/test1", [("/test3", "==")])
+    test2 = manifestitem.RefTest(s2, "/test2", [("/test1", "==")])
+    m.add(test1)
+    m.add(test2)
+
+    m.update_reftests()
+
+    assert m.reftest_nodes == {'test1': {test1},
+                               'test2': {test2}}
+
+    assert list(m) == [("test2", {test2})]
+    assert list(m.local_changes.itertypes()) == []