Merge pull request #61 from w3c/manifest_reftest_diff

Change the way that local changes to reftests are stored in the manifest.

manifest/manifest.py

@@ -42,6 +42,9 @@ def _included_items(self, include_types=None):
                 for path in self.local_changes.iterdeleted():
                     if path in paths:
                         del paths[path]
+                if item_type == "reftest":
+                    for path, items in self.local_changes.iterdeletedreftests():
+                        paths[path] -= items
 
             yield item_type, paths
 
@@ -170,33 +173,51 @@ def update(self,
         self.url_base = url_base
 
     def update_reftests(self):
-        reftest_nodes = self.reftest_nodes.copy()
-        for path, items in self.local_changes.reftest_nodes.iteritems():
-            reftest_nodes[path] |= items
+        default_reftests = self.compute_reftests(self.reftest_nodes)
+        all_reftest_nodes = self.reftest_nodes.copy()
+        all_reftest_nodes.update(self.local_changes.reftest_nodes)
 
-        #TODO: remove locally deleted files
-        tests = set()
-        for items in reftest_nodes.values():
-            tests |= set(item for item in items if not item.is_reference)
+        for item in self.local_changes.iterdeleted():
+            if item in all_reftest_nodes:
+                del all_reftest_nodes[item]
+
+        modified_reftests = self.compute_reftests(all_reftest_nodes)
+
+        added_reftests = modified_reftests - default_reftests
+        # The interesting case here is not when the file is deleted,
+        # but when a reftest like A == B is changed to the form
+        # C == A == B, so that A still exists but is now a ref rather than
+        # a test.
+        removed_reftests = default_reftests - modified_reftests
+
+        dests = [(default_reftests, self._data["reftest"]),
+                 (added_reftests, self.local_changes._data["reftest"]),
+                 (removed_reftests, self.local_changes._deleted_reftests)]
+
+        #TODO: Warn if there exist unreachable reftest nodes
+        for source, target in dests:
+            for item in source:
+                target[item.path].add(item)
+
+    def compute_reftests(self, reftest_nodes):
+        """Given a set of reftest_nodes, return a set of all the nodes that are top-level
+        tests i.e. don't have any incoming reference links."""
+
+        reftests = set()
 
         has_inbound = set()
         for path, items in reftest_nodes.iteritems():
             for item in items:
                 for ref_url, ref_type in item.references:
                     has_inbound.add(ref_url)
 
-        if self.local_changes.reftest_nodes:
-            target = self.local_changes
-        else:
-            target = self
-
-        #TODO: Warn if there exist unreachable reftest nodes
-
         for path, items in reftest_nodes.iteritems():
             for item in items:
                 if item.url in has_inbound:
                     continue
-                target._data["reftest"][path].add(item)
+                reftests.add(item)
+
+        return reftests
 
     def to_json(self):
         out_items = {
@@ -261,13 +282,15 @@ def from_json(cls, tests_root, obj):
                                                     source_files=source_files)
         return self
 
+
 class LocalChanges(object):
     def __init__(self, manifest):
         self.manifest = manifest
         self._data = dict((item_type, defaultdict(set)) for item_type in item_types)
         self._deleted = set()
         self.reftest_nodes = defaultdict(set)
         self.reftest_nodes_by_url = {}
+        self._deleted_reftests = defaultdict(set)
 
     def add(self, item):
         if item is None:
@@ -305,16 +328,24 @@ def iterdeleted(self):
         for item in self._deleted:
             yield item
 
+    def iterdeletedreftests(self):
+        for item in self._deleted_reftests.iteritems():
+            yield item
+
     def __getitem__(self, item_type):
         return self._data[item_type]
 
     def to_json(self):
         reftest_nodes = {from_os_path(key): [v.to_json() for v in value]
                          for key, value in self.reftest_nodes.iteritems()}
 
+        deleted_reftests = {from_os_path(key): [v.to_json() for v in value]
+                            for key, value in self._deleted_reftests.iteritems()}
+
         rv = {"items": defaultdict(dict),
               "reftest_nodes": reftest_nodes,
-              "deleted": [from_os_path(path) for path in self._deleted]}
+              "deleted": [from_os_path(path) for path in self._deleted],
+              "deleted_reftests": deleted_reftests}
 
         for test_type, paths in self._data.iteritems():
             for path, tests in paths.iteritems():
@@ -355,6 +386,13 @@ def from_json(cls, manifest, tests_root, obj, source_files=None):
         for item in obj["deleted"]:
             self.add_deleted(to_os_path(item))
 
+        for path, values in obj.get("deleted_reftests", {}).iteritems():
+            path = to_os_path(path)
+            for v in values:
+                item = RefTest.from_json(self.manifest, tests_root, v,
+                                         source_files=source_files)
+                self._deleted_reftests[path].add(item)
+
         return self
 
 def load(tests_root, manifest):

manifest/tests/test_manifest.py

@@ -0,0 +1,55 @@
+from .. import manifest, item as manifestitem, sourcefile
+
+
+def test_local_reftest_add():
+    m = manifest.Manifest()
+    s = sourcefile.SourceFile("/", "test", "/")
+    test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
+    m.local_changes.add(test)
+    assert list(m) == [(test.path, {test})]
+
+
+def test_local_reftest_delete_path():
+    m = manifest.Manifest()
+    s = sourcefile.SourceFile("/", "test", "/")
+    test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
+    m.add(test)
+    m.local_changes.add_deleted(test.path)
+    assert list(m) == []
+
+
+def test_local_reftest_adjusted():
+    m = manifest.Manifest()
+    s = sourcefile.SourceFile("/", "test", "/")
+    test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
+    m.add(test)
+
+    assert list(m) == [(test.path, {test})]
+
+    assert m.compute_reftests({test.path: {test}}) == {test}
+
+    test_1 = manifestitem.RefTest(s, "/test-1", [("/test", "==")])
+    m.local_changes.add(test_1)
+
+    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})]
+
+
+def test_manifest_to_json():
+    m = manifest.Manifest()
+    s = sourcefile.SourceFile("/", "test", "/")
+    test = manifestitem.RefTest(s, "/test", [("/ref", "==")])
+    m.add(test)
+    test_1 = manifestitem.RefTest(s, "/test-1", [("/test", "==")])
+    m.local_changes.add(test_1)
+    m.local_changes._deleted_reftests[test.path] = {test}
+
+    json_str = m.to_json()
+    loaded = manifest.Manifest.from_json("/", json_str)
+
+    assert list(loaded) == list(m)
+
+    assert loaded.to_json() == json_str