Andrew Godwin avatar Andrew Godwin committed 0db034c

Bugfree backwards-compat comparison, with the aid of unit tests.

Comments (0)

Files changed (3)

south/management/commands/startmigration.py

 # Backwards-compat comparison that ignores orm. on the RHS and not the left
 # and which knows django.db.models.fields.CharField = models.CharField
 def different_attributes(old, new):
+    
     # If they're not triples, just do normal comparison
-    if not isinstance(old, (list, tuple)) or not isinstance(new, (list, tuple)):
+    if not isinstance(old, (list, tuple)) or not isinstance(new, (list, tuple)) \
+       or len(old) != 3 or len(new) != 3:
         return old != new
+    
+    # Expand them out into parts
+    old_field, old_pos, old_kwd = old
+    new_field, new_pos, new_kwd = new
+    
+    # Copy the positional and keyword arguments so we can compare them and pop off things
+    old_pos, new_pos = old_pos[:], new_pos[:]
+    old_kwd = dict(old_kwd.items())
+    new_kwd = dict(new_kwd.items())
+    
     # If the first bit is different, check it's not by dj.db.models...
-    if old[0] != new[0]:
-        if old[0].startswith("models.") and (new[0].startswith("django.db.models") \
-         or new[0].startswith("django.contrib.gis")):
-            if old[0].split(".")[-1] != new[0].split(".")[-1]:
+    if old_field != new_field:
+        if old_field.startswith("models.") and (new_field.startswith("django.db.models") \
+         or new_field.startswith("django.contrib.gis")):
+            if old_field.split(".")[-1] != new_field.split(".")[-1]:
                 return True
-    # If the third bits or end of second are different, it really is different.
-    if old[2] != new[2] or old[1][1:] != new[1][1:]:
-        return True
-    if not old[1] and not new[1]:
-        return False
-    # Compare first positional arg
-    if new[1] and old[1]:
-        if "orm" in new[1][0] and "orm" not in old[1][0]:
-            # Do special comparison to fix #153
-            try:
-                return old[1][0] != new[1][0].split("'")[1].split(".")[1]
-            except IndexError:
-                pass # Fall back to next comparison
-        return old[1][0] != new[1][0]
-    else:
-        return old != new
+            else:
+                # Remove those fields from the final comparison
+                old_field = new_field = ""
+    
+    # If there's a positional argument in the first, and a 'to' in the second,
+    # see if they're actually comparable.
+    if (old_pos and "to" in new_kwd) and ("orm" in new_kwd['to'] and "orm" not in old_pos[0]):
+        # Do special comparison to fix #153
+        try:
+            if old_pos[0] != new_kwd['to'].split("'")[1].split(".")[1]:
+                return True
+        except IndexError:
+            pass # Fall back to next comparison
+        # Remove those attrs from the final comparison
+        old_pos = old_pos[1:]
+        del new_kwd['to']
+    
+    return old_field != new_field or old_pos != new_pos or old_kwd != new_kwd
     
     
 

south/tests/__init__.py

 
 if not skiptest:
     from south.tests.db import *
-    from south.tests.logic import *
+    from south.tests.logic import *
+    from south.tests.autodetection import *

south/tests/autodetection.py

+import unittest
+
+from south.management.commands import startmigration
+
+class TestComparison(unittest.TestCase):
+    
+    """
+    Tests the comparison methods of startmigration.
+    """
+    
+    def test_no_change(self):
+        "Test with a completely unchanged definition."
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['southdemo.Lizard']"}),
+                ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['southdemo.Lizard']"}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.related.ForeignKey', ['ohhai', 'there'], {'to': "somewhere", "from": "there"}),
+                ('django.db.models.fields.related.ForeignKey', ['ohhai', 'there'], {"from": "there", 'to': "somewhere"}),
+            ),
+            False,
+        )
+    
+    
+    def test_pos_change(self):
+        "Test with a changed positional argument."
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['hi'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', [], {'to': "foo"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', [], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['bye'], {'to': "foo"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['pi'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['pi'], {'to': "foo"}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['pisdadad'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['pi'], {'to': "foo"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['hi'], {}),
+                ('django.db.models.fields.CharField', [], {}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', [], {}),
+                ('django.db.models.fields.CharField', ['bye'], {}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['pi'], {}),
+                ('django.db.models.fields.CharField', ['pi'], {}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['pi'], {}),
+                ('django.db.models.fields.CharField', ['45fdfdf'], {}),
+            ),
+            True,
+        )
+    
+    
+    def test_kwd_change(self):
+        "Test a changed keyword argument"
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['pi'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['pi'], {'to': "blue"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', [], {'to': "foo"}),
+                ('django.db.models.fields.CharField', [], {'to': "blue"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['b'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['b'], {'to': "blue"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', [], {'to': "foo"}),
+                ('django.db.models.fields.CharField', [], {}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['a'], {'to': "foo"}),
+                ('django.db.models.fields.CharField', ['a'], {}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', [], {}),
+                ('django.db.models.fields.CharField', [], {'to': "foo"}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('django.db.models.fields.CharField', ['a'], {}),
+                ('django.db.models.fields.CharField', ['a'], {'to': "foo"}),
+            ),
+            True,
+        )
+        
+    
+    
+    def test_backcompat_nochange(self):
+        "Test that the backwards-compatable comparison is working"
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', [], {}),
+                ('django.db.models.fields.CharField', [], {}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['ack'], {}),
+                ('django.db.models.fields.CharField', ['ack'], {}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', [], {'to':'b'}),
+                ('django.db.models.fields.CharField', [], {'to':'b'}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {'to':'you'}),
+                ('django.db.models.fields.CharField', ['hah'], {'to':'you'}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {'to':'you'}),
+                ('django.db.models.fields.CharField', ['hah'], {'to':'heh'}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {}),
+                ('django.db.models.fields.CharField', [], {'to':"orm['appname.hah']"}),
+            ),
+            False,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {}),
+                ('django.db.models.fields.CharField', [], {'to':'hah'}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {}),
+                ('django.db.models.fields.CharField', [], {'to':'rrr'}),
+            ),
+            True,
+        )
+        
+        self.assertEqual(
+            startmigration.different_attributes(
+                ('models.CharField', ['hah'], {}),
+                ('django.db.models.fields.IntField', [], {'to':'hah'}),
+            ),
+            True,
+        )
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.