Merge branch 'mrhadenough-fix_optimization_bugs'

3 files changed, 520 insertions, 29 deletions

diff --git a/jsonpatch.py b/jsonpatch.py
index a9574ae..0116ca2 100644
--- a/jsonpatch.py
+++ b/jsonpatch.py
@@ -171,6 +171,14 @@ def make_patch(src, dst):
     >>> new == dst
     True
     """
+
+    # TODO: fix patch optimiztion and remove the following check
+    # fix when patch with optimization is incorrect
+    patch = JsonPatch.from_diff(src, dst)
+    new = patch.apply(src)
+    if new != dst:
+        return JsonPatch.from_diff(src, dst, False)
+
     return JsonPatch.from_diff(src, dst)
 
 
@@ -268,7 +276,7 @@ class JsonPatch(object):
         return cls(patch)
 
     @classmethod
-    def from_diff(cls, src, dst):
+    def from_diff(cls, src, dst, optimization=True):
         """Creates JsonPatch instance based on comparing of two document
         objects. Json patch would be created for `src` argument against `dst`
         one.
@@ -320,7 +328,7 @@ class JsonPatch(object):
                            'value': dst[key]}
 
         def compare_lists(path, src, dst):
-            return _compare_lists(path, src, dst)
+            return _compare_lists(path, src, dst, optimization=optimization)
 
         return cls(list(compare_values([], src, dst)))
 
@@ -561,9 +569,12 @@ class CopyOperation(PatchOperation):
         return obj
 
 
-def _compare_lists(path, src, dst):
+def _compare_lists(path, src, dst, optimization=True):
     """Compares two lists objects and return JSON patch about."""
-    return _optimize(_compare(path, src, dst, *_split_by_common_seq(src, dst)))
+    patch = list(_compare(path, src, dst, *_split_by_common_seq(src, dst)))
+    if optimization:
+        return list(_optimize(patch))
+    return patch
 
 
 def _longest_common_subseq(src, dst):
@@ -770,7 +781,16 @@ def _optimize_using_replace(prev, cur):
     if cur['op'] == 'add':
         # make recursive patch
         patch = make_patch(prev['value'], cur['value'])
-        if len(patch.patch) == 1 and patch.patch[0]['op'] != 'remove':
+        # check case when dict "remove" is less than "add" and has a same key
+        if isinstance(prev['value'], dict) and isinstance(cur['value'], dict) and len(prev['value'].keys()) == 1:
+            prev_set = set(prev['value'].keys())
+            cur_set = set(cur['value'].keys())
+            if prev_set & cur_set == prev_set:
+                patch = make_patch(cur['value'], prev['value'])
+
+        if len(patch.patch) == 1 and \
+                patch.patch[0]['op'] != 'remove' and \
+                patch.patch[0]['path'] and patch.patch[0]['path'].split('/')[1] in prev['value']:
             prev['path'] = prev['path'] + patch.patch[0]['path']
             prev['value'] = patch.patch[0]['value']
         else:
diff --git a/tests.js b/tests.js
new file mode 100644
index 0000000..a09a8d6
--- /dev/null
+++ b/tests.js
@@ -0,0 +1,398 @@
+[
+    { "comment": "empty list, empty docs",
+      "doc": {},
+      "patch": [],
+      "expected": {} },
+
+    { "comment": "empty patch list",
+      "doc": {"foo": 1},
+      "patch": [],
+      "expected": {"foo": 1} },
+
+    { "comment": "rearrangements OK?",
+      "doc": {"foo": 1, "bar": 2},
+      "patch": [],
+      "expected": {"bar":2, "foo": 1} },
+
+    { "comment": "rearrangements OK?  How about one level down ... array",
+      "doc": [{"foo": 1, "bar": 2}],
+      "patch": [],
+      "expected": [{"bar":2, "foo": 1}] },
+
+    { "comment": "rearrangements OK?  How about one level down...",
+      "doc": {"foo":{"foo": 1, "bar": 2}},
+      "patch": [],
+      "expected": {"foo":{"bar":2, "foo": 1}} },
+
+    { "comment": "add replaces any existing field",
+      "doc": {"foo": null},
+      "patch": [{"op": "add", "path": "/foo", "value":1}],
+      "expected": {"foo": 1} },
+
+    { "comment": "toplevel array",
+      "doc": [],
+      "patch": [{"op": "add", "path": "/0", "value": "foo"}],
+      "expected": ["foo"] },
+
+    { "comment": "toplevel array, no change",
+      "doc": ["foo"],
+      "patch": [],
+      "expected": ["foo"] },
+
+    { "comment": "toplevel object, numeric string",
+      "doc": {},
+      "patch": [{"op": "add", "path": "/foo", "value": "1"}],
+      "expected": {"foo":"1"} },
+
+    { "comment": "toplevel object, integer",
+      "doc": {},
+      "patch": [{"op": "add", "path": "/foo", "value": 1}],
+      "expected": {"foo":1} },
+
+    { "comment": "Toplevel scalar values OK?",
+      "doc": "foo",
+      "patch": [{"op": "replace", "path": "", "value": "bar"}],
+      "expected": "bar",
+      "disabled": true },
+
+    { "comment": "Add, / target",
+      "doc": {},
+      "patch": [ {"op": "add", "path": "/", "value":1 } ],
+      "expected": {"":1} },
+
+    { "comment": "Add, /foo/ deep target (trailing slash)",
+      "doc": {"foo": {}},
+      "patch": [ {"op": "add", "path": "/foo/", "value":1 } ],
+      "expected": {"foo":{"": 1}} },
+
+    { "comment": "Add composite value at top level",
+      "doc": {"foo": 1},
+      "patch": [{"op": "add", "path": "/bar", "value": [1, 2]}],
+      "expected": {"foo": 1, "bar": [1, 2]} },
+
+    { "comment": "Add into composite value",
+      "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "add", "path": "/baz/0/foo", "value": "world"}],
+      "expected": {"foo": 1, "baz": [{"qux": "hello", "foo": "world"}]} },
+
+    { "doc": {"bar": [1, 2]},
+      "patch": [{"op": "add", "path": "/bar/8", "value": "5"}],
+      "error": "Out of bounds (upper)" },
+
+    { "doc": {"bar": [1, 2]},
+      "patch": [{"op": "add", "path": "/bar/-1", "value": "5"}],
+      "error": "Out of bounds (lower)" },
+
+    { "doc": {"foo": 1},
+      "patch": [{"op": "add", "path": "/bar", "value": true}],
+      "expected": {"foo": 1, "bar": true} },
+
+    { "doc": {"foo": 1},
+      "patch": [{"op": "add", "path": "/bar", "value": false}],
+      "expected": {"foo": 1, "bar": false} },
+
+    { "doc": {"foo": 1},
+      "patch": [{"op": "add", "path": "/bar", "value": null}],
+      "expected": {"foo": 1, "bar": null} },
+
+    { "comment": "0 can be an array index or object element name",
+      "doc": {"foo": 1},
+      "patch": [{"op": "add", "path": "/0", "value": "bar"}],
+      "expected": {"foo": 1, "0": "bar" } },
+
+    { "doc": ["foo"],
+      "patch": [{"op": "add", "path": "/1", "value": "bar"}],
+      "expected": ["foo", "bar"] },
+
+    { "doc": ["foo", "sil"],
+      "patch": [{"op": "add", "path": "/1", "value": "bar"}],
+      "expected": ["foo", "bar", "sil"] },
+
+    { "doc": ["foo", "sil"],
+      "patch": [{"op": "add", "path": "/0", "value": "bar"}],
+      "expected": ["bar", "foo", "sil"] },
+
+    { "comment": "push item to array via last index + 1",
+      "doc": ["foo", "sil"],
+      "patch": [{"op":"add", "path": "/2", "value": "bar"}],
+      "expected": ["foo", "sil", "bar"] },
+
+    { "comment": "add item to array at index > length should fail",
+      "doc": ["foo", "sil"],
+      "patch": [{"op":"add", "path": "/3", "value": "bar"}],
+      "error": "index is greater than number of items in array" },
+
+    { "comment": "test against implementation-specific numeric parsing",
+      "doc": {"1e0": "foo"},
+      "patch": [{"op": "test", "path": "/1e0", "value": "foo"}],
+      "expected": {"1e0": "foo"} },
+
+    { "comment": "test with bad number should fail",
+      "doc": ["foo", "bar"],
+      "patch": [{"op": "test", "path": "/1e0", "value": "bar"}],
+      "error": "test op shouldn't get array element 1" },
+
+    { "doc": ["foo", "sil"],
+      "patch": [{"op": "add", "path": "/bar", "value": 42}],
+      "error": "Object operation on array target" },
+
+    { "doc": ["foo", "sil"],
+      "patch": [{"op": "add", "path": "/1", "value": ["bar", "baz"]}],
+      "expected": ["foo", ["bar", "baz"], "sil"],
+      "comment": "value in array add not flattened" },
+
+    { "doc": {"foo": 1, "bar": [1, 2, 3, 4]},
+      "patch": [{"op": "remove", "path": "/bar"}],
+      "expected": {"foo": 1} },
+
+    { "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "remove", "path": "/baz/0/qux"}],
+      "expected": {"foo": 1, "baz": [{}]} },
+
+    { "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "replace", "path": "/foo", "value": [1, 2, 3, 4]}],
+      "expected": {"foo": [1, 2, 3, 4], "baz": [{"qux": "hello"}]} },
+
+    { "doc": {"foo": [1, 2, 3, 4], "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "replace", "path": "/baz/0/qux", "value": "world"}],
+      "expected": {"foo": [1, 2, 3, 4], "baz": [{"qux": "world"}]} },
+
+    { "doc": ["foo"],
+      "patch": [{"op": "replace", "path": "/0", "value": "bar"}],
+      "expected": ["bar"] },
+
+    { "doc": [""],
+      "patch": [{"op": "replace", "path": "/0", "value": 0}],
+      "expected": [0] },
+
+    { "doc": [""],
+      "patch": [{"op": "replace", "path": "/0", "value": true}],
+      "expected": [true] },
+
+    { "doc": [""],
+      "patch": [{"op": "replace", "path": "/0", "value": false}],
+      "expected": [false] },
+
+    { "doc": [""],
+      "patch": [{"op": "replace", "path": "/0", "value": null}],
+      "expected": [null] },
+
+    { "doc": ["foo", "sil"],
+      "patch": [{"op": "replace", "path": "/1", "value": ["bar", "baz"]}],
+      "expected": ["foo", ["bar", "baz"]],
+      "comment": "value in array replace not flattened" },
+
+    { "comment": "replace whole document",
+      "doc": {"foo": "bar"},
+      "patch": [{"op": "replace", "path": "", "value": {"baz": "qux"}}],
+      "expected": {"baz": "qux"} },
+
+    { "comment": "spurious patch properties",
+      "doc": {"foo": 1},
+      "patch": [{"op": "test", "path": "/foo", "value": 1, "spurious": 1}],
+      "expected": {"foo": 1} },
+
+    { "doc": {"foo": null},
+      "patch": [{"op": "test", "path": "/foo", "value": null}],
+      "comment": "null value should be valid obj property" },
+
+    { "doc": {"foo": null},
+      "patch": [{"op": "replace", "path": "/foo", "value": "truthy"}],
+      "expected": {"foo": "truthy"},
+      "comment": "null value should be valid obj property to be replaced with something truthy" },
+
+    { "doc": {"foo": null},
+      "patch": [{"op": "move", "from": "/foo", "path": "/bar"}],
+      "expected": {"bar": null},
+      "comment": "null value should be valid obj property to be moved" },
+
+    { "doc": {"foo": null},
+      "patch": [{"op": "copy", "from": "/foo", "path": "/bar"}],
+      "expected": {"foo": null, "bar": null},
+      "comment": "null value should be valid obj property to be copied" },
+
+    { "doc": {"foo": null},
+      "patch": [{"op": "remove", "path": "/foo"}],
+      "expected": {},
+      "comment": "null value should be valid obj property to be removed" },
+
+    { "doc": {"foo": "bar"},
+      "patch": [{"op": "replace", "path": "/foo", "value": null}],
+      "expected": {"foo": null},
+      "comment": "null value should still be valid obj property replace other value" },
+
+    { "doc": {"foo": {"foo": 1, "bar": 2}},
+      "patch": [{"op": "test", "path": "/foo", "value": {"bar": 2, "foo": 1}}],
+      "comment": "test should pass despite rearrangement" },
+
+    { "doc": {"foo": [{"foo": 1, "bar": 2}]},
+      "patch": [{"op": "test", "path": "/foo", "value": [{"bar": 2, "foo": 1}]}],
+      "comment": "test should pass despite (nested) rearrangement" },
+
+    { "doc": {"foo": {"bar": [1, 2, 5, 4]}},
+      "patch": [{"op": "test", "path": "/foo", "value": {"bar": [1, 2, 5, 4]}}],
+      "comment": "test should pass - no error" },
+
+    { "doc": {"foo": {"bar": [1, 2, 5, 4]}},
+      "patch": [{"op": "test", "path": "/foo", "value": [1, 2]}],
+      "error": "test op should fail" },
+
+    { "comment": "Whole document",
+      "doc": { "foo": 1 },
+      "patch": [{"op": "test", "path": "", "value": {"foo": 1}}],
+      "disabled": true },
+
+    { "comment": "Empty-string element",
+      "doc": { "": 1 },
+      "patch": [{"op": "test", "path": "/", "value": 1}] },
+
+    { "doc": {
+            "foo": ["bar", "baz"],
+            "": 0,
+            "a/b": 1,
+            "c%d": 2,
+            "e^f": 3,
+            "g|h": 4,
+            "i\\j": 5,
+            "k\"l": 6,
+            " ": 7,
+            "m~n": 8
+            },
+      "patch": [{"op": "test", "path": "/foo", "value": ["bar", "baz"]},
+                {"op": "test", "path": "/foo/0", "value": "bar"},
+                {"op": "test", "path": "/", "value": 0},
+                {"op": "test", "path": "/a~1b", "value": 1},
+                {"op": "test", "path": "/c%d", "value": 2},
+                {"op": "test", "path": "/e^f", "value": 3},
+                {"op": "test", "path": "/g|h", "value": 4},
+                {"op": "test", "path":  "/i\\j", "value": 5},
+                {"op": "test", "path": "/k\"l", "value": 6},
+                {"op": "test", "path": "/ ", "value": 7},
+                {"op": "test", "path": "/m~0n", "value": 8}] },
+
+    { "comment": "Move to same location has no effect",
+      "doc": {"foo": 1},
+      "patch": [{"op": "move", "from": "/foo", "path": "/foo"}],
+      "expected": {"foo": 1} },
+
+    { "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "move", "from": "/foo", "path": "/bar"}],
+      "expected": {"baz": [{"qux": "hello"}], "bar": 1} },
+
+    { "doc": {"baz": [{"qux": "hello"}], "bar": 1},
+      "patch": [{"op": "move", "from": "/baz/0/qux", "path": "/baz/1"}],
+      "expected": {"baz": [{}, "hello"], "bar": 1} },
+
+    { "doc": {"baz": [{"qux": "hello"}], "bar": 1},
+      "patch": [{"op": "copy", "from": "/baz/0", "path": "/boo"}],
+      "expected": {"baz":[{"qux":"hello"}],"bar":1,"boo":{"qux":"hello"}} },
+
+    { "comment": "replacing the root of the document is possible with add",
+      "doc": {"foo": "bar"},
+      "patch": [{"op": "add", "path": "", "value": {"baz": "qux"}}],
+      "expected": {"baz":"qux"}},
+
+    { "comment": "Adding to \"/-\" adds to the end of the array",
+      "doc": [ 1, 2 ],
+      "patch": [ { "op": "add", "path": "/-", "value": { "foo": [ "bar", "baz" ] } } ],
+      "expected": [ 1, 2, { "foo": [ "bar", "baz" ] } ]},
+
+    { "comment": "Adding to \"/-\" adds to the end of the array, even n levels down",
+      "doc": [ 1, 2, [ 3, [ 4, 5 ] ] ],
+      "patch": [ { "op": "add", "path": "/2/1/-", "value": { "foo": [ "bar", "baz" ] } } ],
+      "expected": [ 1, 2, [ 3, [ 4, 5, { "foo": [ "bar", "baz" ] } ] ] ]},
+
+    { "comment": "test remove with bad number should fail",
+      "doc": {"foo": 1, "baz": [{"qux": "hello"}]},
+      "patch": [{"op": "remove", "path": "/baz/1e0/qux"}],
+      "error": "remove op shouldn't remove from array with bad number" },
+
+    { "comment": "test remove on array",
+      "doc": [1, 2, 3, 4],
+      "patch": [{"op": "remove", "path": "/0"}],
+      "expected": [2, 3, 4] },
+
+    { "comment": "test repeated removes",
+      "doc": [1, 2, 3, 4],
+      "patch": [{ "op": "remove", "path": "/1" },
+                { "op": "remove", "path": "/2" }],
+      "expected": [1, 3] },
+
+    { "comment": "test remove with bad index should fail",
+      "doc": [1, 2, 3, 4],
+      "patch": [{"op": "remove", "path": "/1e0"}],
+      "error": "remove op shouldn't remove from array with bad number" },
+
+    { "comment": "test replace with bad number should fail",
+      "doc": [""],
+      "patch": [{"op": "replace", "path": "/1e0", "value": false}],
+      "error": "replace op shouldn't replace in array with bad number" },
+
+    { "comment": "test copy with bad number should fail",
+      "doc": {"baz": [1,2,3], "bar": 1},
+      "patch": [{"op": "copy", "from": "/baz/1e0", "path": "/boo"}],
+      "error": "copy op shouldn't work with bad number" },
+
+    { "comment": "test move with bad number should fail",
+      "doc": {"foo": 1, "baz": [1,2,3,4]},
+      "patch": [{"op": "move", "from": "/baz/1e0", "path": "/foo"}],
+      "error": "move op shouldn't work with bad number" },
+
+    { "comment": "test add with bad number should fail",
+      "doc": ["foo", "sil"],
+      "patch": [{"op": "add", "path": "/1e0", "value": "bar"}],
+      "error": "add op shouldn't add to array with bad number" },
+
+    { "comment": "missing 'value' parameter to add",
+      "doc": [ 1 ],
+      "patch": [ { "op": "add", "path": "/-" } ],
+      "error": "missing 'value' parameter" },
+
+    { "comment": "missing 'value' parameter to replace",
+      "doc": [ 1 ],
+      "patch": [ { "op": "replace", "path": "/0" } ],
+      "error": "missing 'value' parameter" },
+
+    { "comment": "missing 'value' parameter to test",
+      "doc": [ null ],
+      "patch": [ { "op": "test", "path": "/0" } ],
+      "error": "missing 'value' parameter" },
+
+    { "comment": "missing value parameter to test - where undef is falsy",
+      "doc": [ false ],
+      "patch": [ { "op": "test", "path": "/0" } ],
+      "error": "missing 'value' parameter" },
+
+    { "comment": "missing from parameter to copy",
+      "doc": [ 1 ],
+      "patch": [ { "op": "copy", "path": "/-" } ],
+      "error": "missing 'from' parameter" },
+
+    { "comment": "missing from parameter to move",
+      "doc": { "foo": 1 },
+      "patch": [ { "op": "move", "path": "" } ],
+      "error": "missing 'from' parameter" },
+
+    { "comment": "duplicate ops",
+      "doc": { "foo": "bar" },
+      "patch": [ { "op": "add", "path": "/baz", "value": "qux",
+                   "op": "move", "from":"/foo" } ],
+      "error": "patch has two 'op' members",
+      "disabled": true },
+
+    { "comment": "unrecognized op should fail",
+      "doc": {"foo": 1},
+      "patch": [{"op": "spam", "path": "/foo", "value": 1}],
+      "error": "Unrecognized op 'spam'" },
+
+    { "comment": "test with bad array number that has leading zeros",
+      "doc": ["foo", "bar"],
+      "patch": [{"op": "test", "path": "/00", "value": "foo"}],
+      "error": "test op should reject the array value, it has leading zeros" },
+
+    { "comment": "test with bad array number that has leading zeros",
+      "doc": ["foo", "bar"],
+      "patch": [{"op": "test", "path": "/01", "value": "bar"}],
+      "error": "test op should reject the array value, it has leading zeros" }
+
+]
diff --git a/tests.py b/tests.py
index 5acf24b..51d9517 100755
--- a/tests.py
+++ b/tests.py
@@ -13,6 +13,34 @@ import sys
 
 class ApplyPatchTestCase(unittest.TestCase):
 
+    def test_js_file(self):
+        with open('./tests.js', 'r') as f:
+            tests = json.load(f)
+            for test in tests:
+                try:
+                    if 'expected' not in test:
+                        continue
+                    result = jsonpatch.apply_patch(test['doc'], test['patch'])
+                    self.assertEqual(result, test['expected'])
+                except Exception:
+                    if test.get('error'):
+                        continue
+                    else:
+                        raise
+
+    def test_success_if_replaced_dict(self):
+        src = [{'a': 1}, {'b': 2}]
+        dst = [{'a': 1, 'b': 2}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
+
+    def test_success_if_raise_no_error(self):
+        src = [{}]
+        dst = [{'key': ''}]
+        patch = jsonpatch.make_patch(src, dst)
+        patch.apply(src)
+        self.assertTrue(True)
+
     def test_apply_patch_from_string(self):
         obj = {'foo': 'bar'}
         patch = '[{"op": "add", "path": "/baz", "value": "qux"}]'
@@ -239,7 +267,6 @@ class EqualityTestCase(unittest.TestCase):
         self.assertEqual(json.dumps(patch_obj), patch.to_string())
 
 
-
 class MakePatchTestCase(unittest.TestCase):
 
     def test_apply_patch_to_copy(self):
@@ -308,6 +335,50 @@ class MakePatchTestCase(unittest.TestCase):
         res = jsonpatch.apply_patch(src, patch)
         self.assertEqual(res, dst)
 
+    def test_escape(self):
+        src = {"x/y": 1}
+        dst = {"x/y": 2}
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual([{"path": "/x~1y", "value": 2, "op": "replace"}], patch.patch)
+        res = patch.apply(src)
+        self.assertEqual(res, dst)
+
+    def test_root_list(self):
+        """ Test making and applying a patch of the root is a list """
+        src = [{'foo': 'bar', 'boo': 'qux'}]
+        dst = [{'baz': 'qux', 'foo': 'boo'}]
+        patch = jsonpatch.make_patch(src, dst)
+        res = patch.apply(src)
+        self.assertEqual(res, dst)
+
+    def test_make_patch_unicode(self):
+        """ Test if unicode keys and values are handled correctly """
+        src = {}
+        dst = {'\xee': '\xee'}
+        patch = jsonpatch.make_patch(src, dst)
+        res = patch.apply(src)
+        self.assertEqual(res, dst)
+
+    def test_issue40(self):
+        """ Tests an issue in _split_by_common_seq reported in #40 """
+
+        src = [8, 7, 2, 1, 0, 9, 4, 3, 5, 6]
+        dest = [7, 2, 1, 0, 9, 4, 3, 6, 5, 8]
+        patch = jsonpatch.make_patch(src, dest)
+
+    def test_json_patch(self):
+        old = {
+            'queue': {'teams_out': [{'id': 3, 'reason': 'If tied'}, {'id': 5, 'reason': 'If tied'}]},
+        }
+        new = {
+            'queue': {'teams_out': [{'id': 5, 'reason': 'If lose'}]}
+        }
+        patch = jsonpatch.make_patch(old, new)
+        new_from_patch = jsonpatch.apply_patch(old, patch)
+        self.assertEqual(new, new_from_patch)
+
+
+class OptimizationTests(unittest.TestCase):
     def test_use_replace_instead_of_remove_add(self):
         src = {'foo': [1, 2, 3]}
         dst = {'foo': [3, 2, 3]}
@@ -344,41 +415,42 @@ class MakePatchTestCase(unittest.TestCase):
         res = jsonpatch.apply_patch(src, patch)
         self.assertEqual(res, dst)
 
-    def test_escape(self):
-        src = {"x/y": 1}
-        dst = {"x/y": 2}
+    def test_success_if_replace_inside_dict(self):
+        src = [{'a': 1, 'foo': {'b': 2, 'd': 5}}]
+        dst = [{'a': 1, 'foo': {'b': 3, 'd': 6}}]
         patch = jsonpatch.make_patch(src, dst)
-        self.assertEqual([{"path": "/x~1y", "value": 2, "op": "replace"}], patch.patch)
-        res = patch.apply(src)
-        self.assertEqual(res, dst)
+        self.assertEqual(patch.apply(src), dst)
 
-    def test_root_list(self):
-        """ Test making and applying a patch of the root is a list """
-        src = [{'foo': 'bar', 'boo': 'qux'}]
-        dst = [{'baz': 'qux', 'foo': 'boo'}]
+    def test_success_if_replace_single_value(self):
+        src = [{'a': 1, 'b': 2, 'd': 5}]
+        dst = [{'a': 1, 'c': 3, 'd': 5}]
         patch = jsonpatch.make_patch(src, dst)
-        res = patch.apply(src)
-        self.assertEqual(res, dst)
+        self.assertEqual(patch.apply(src), dst)
 
-    def test_make_patch_unicode(self):
-        """ Test if unicode keys and values are handled correctly """
-        src = {}
-        dst = {'\xee': '\xee'}
+    def test_success_if_replaced_by_object(self):
+        src = [{'a': 1, 'b': 2, 'd': 5}]
+        dst = [{'d': 6}]
         patch = jsonpatch.make_patch(src, dst)
-        res = patch.apply(src)
-        self.assertEqual(res, dst)
+        self.assertEqual(patch.apply(src), dst)
 
-    def test_issue40(self):
-        """ Tests an issue in _split_by_common_seq reported in #40 """
+    def test_success_if_correct_patch_appied(self):
+        src = [{'a': 1}, {'b': 2}]
+        dst = [{'a': 1, 'b': 2}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
 
-        src = [8, 7, 2, 1, 0, 9, 4, 3, 5, 6]
-        dest = [7, 2, 1, 0, 9, 4, 3, 6, 5, 8]
-        patch = jsonpatch.make_patch(src, dest)
+    def test_success_if_correct_expected_patch_appied(self):
+        src = [{"a": 1, "b": 2}]
+        dst = [{"b": 2, "c": 2}]
+        exp = [{'path': '/0', 'value': {'c': 2, 'b': 2}, 'op': 'replace'}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.patch, exp)
 
     def test_minimal_patch(self):
         """ Test whether a minimal patch is created, see #36 """
         src = [{"foo": 1, "bar": 2}]
         dst = [{"foo": 2, "bar": 2}]
+
         patch = jsonpatch.make_patch(src, dst)
 
         exp = [
@@ -458,6 +530,7 @@ if __name__ == '__main__':
         suite.addTest(unittest.makeSuite(MakePatchTestCase))
         suite.addTest(unittest.makeSuite(InvalidInputTests))
         suite.addTest(unittest.makeSuite(ConflictTests))
+        suite.addTest(unittest.makeSuite(OptimizationTests))
         return suite