From fcb6c2bb9ae17663047134d41befaac7b45eb048 Mon Sep 17 00:00:00 2001
From: Nyall Dawson <nyall.dawson@gmail.com>
Date: Tue, 8 Nov 2016 12:35:25 +1000
Subject: [PATCH] Fix some incorrect return values from QgsFeature python
 bindings

---
 python/core/qgsfeature.sip          |  4 ++++
 tests/src/python/test_qgsfeature.py | 29 +++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/python/core/qgsfeature.sip b/python/core/qgsfeature.sip
index 83fb397b81b..752bc6f5de9 100644
--- a/python/core/qgsfeature.sip
+++ b/python/core/qgsfeature.sip
@@ -293,6 +293,8 @@ class QgsFeature
     PyErr_SetString(PyExc_KeyError, QByteArray::number(a0));
     sipIsErr = 1;
   }
+
+  sipRes = rv;
 %End
 
     /** Initialize this feature with the given number of fields. Discard any previously set attribute data.
@@ -412,10 +414,12 @@ class QgsFeature
   {
     PyErr_SetString(PyExc_KeyError, a0->toAscii());
     sipIsErr = 1;
+    sipRes = false;
   }
   else
   {
     sipCpp->deleteAttribute( fieldIdx );
+    sipRes = true;
   }
 %End
     /** Lookup attribute value from attribute name. Field map must be associated using @link setFields @endlink
diff --git a/tests/src/python/test_qgsfeature.py b/tests/src/python/test_qgsfeature.py
index 50327899ce4..dd5d05a6738 100644
--- a/tests/src/python/test_qgsfeature.py
+++ b/tests/src/python/test_qgsfeature.py
@@ -15,7 +15,7 @@ __revision__ = '$Format:%H$'
 import qgis  # NOQA
 
 import os
-from qgis.core import QgsFeature, QgsGeometry, QgsPoint, QgsVectorLayer, NULL
+from qgis.core import QgsFeature, QgsGeometry, QgsPoint, QgsVectorLayer, NULL, QgsFields, QgsField
 from qgis.testing import start_app, unittest
 from utilities import unitTestDataPath
 
@@ -66,13 +66,22 @@ class TestQgsFeature(unittest.TestCase):
 
         assert myAttributes == myExpectedAttributes, myMessage
 
-    def test_SetAttribute(self):
+    def test_SetAttributes(self):
         feat = QgsFeature()
         feat.initAttributes(1)
         feat.setAttributes([0])
         feat.setAttributes([NULL])
         assert [NULL] == feat.attributes()
 
+    def test_setAttribute(self):
+        feat = QgsFeature()
+        feat.initAttributes(1)
+        with self.assertRaises(KeyError):
+            feat.setAttribute(-1, 5)
+        with self.assertRaises(KeyError):
+            feat.setAttribute(10, 5)
+        self.assertTrue(feat.setAttribute(0, 5))
+
     def test_DeleteAttribute(self):
         feat = QgsFeature()
         feat.initAttributes(3)
@@ -85,6 +94,22 @@ class TestQgsFeature(unittest.TestCase):
         myMessage = '\nExpected: %s\nGot: %s' % (str(myExpectedAttrs), str(myAttrs))
         assert myAttrs == myExpectedAttrs, myMessage
 
+    def test_DeleteAttributeByName(self):
+        fields = QgsFields()
+        field1 = QgsField('my_field')
+        fields.append(field1)
+        field2 = QgsField('my_field2')
+        fields.append(field2)
+
+        feat = QgsFeature(fields)
+        feat.initAttributes(2)
+        feat[0] = "text1"
+        feat[1] = "text2"
+        with self.assertRaises(KeyError):
+            feat.deleteAttribute('not present')
+        self.assertTrue(feat.deleteAttribute('my_field'))
+        self.assertEqual(feat.attributes(), ['text2'])
+
     def test_SetGeometry(self):
         feat = QgsFeature()
         feat.setGeometry(QgsGeometry.fromPoint(QgsPoint(123, 456)))