From 186ec99eb9a5580bbd6404ba61b5210f524d65aa Mon Sep 17 00:00:00 2001
From: Tao Bao <tbao@google.com>
Date: Sat, 23 Dec 2017 11:50:52 -0800
Subject: [PATCH] releasetools: Fix a bug in blockimgdiff.HeapItem.

HeapItem defines __bool__(), which contains a logical error that should
return the opposite value.

Note that the bug only manifests while using Python 3, which calls
__bool__(). With Python 2, `if x:` or bool(x) actually calls
x.__nonzero__() or x.__len__(). If a class defines neither __len__() nor
__nonzero__(), as the case in HeapItem, it always returns True.

Test: python -m unittest test_blockimgdiff
Test: python3 -m unittest test_blockimgdiff
Test: Generate an incremental non-A/B OTA package successfully.
Change-Id: Ibe8430e0b495a7d2f430cfffb716d2536ffb53d2
---
 tools/releasetools/blockimgdiff.py      | 14 +++++++---
 tools/releasetools/test_blockimgdiff.py | 35 +++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/tools/releasetools/blockimgdiff.py b/tools/releasetools/blockimgdiff.py
index 8f06b95f8..69750b28c 100644
--- a/tools/releasetools/blockimgdiff.py
+++ b/tools/releasetools/blockimgdiff.py
@@ -237,15 +237,23 @@ class Transfer(object):
 class HeapItem(object):
   def __init__(self, item):
     self.item = item
-    # Negate the score since python's heap is a min-heap and we want
-    # the maximum score.
+    # Negate the score since python's heap is a min-heap and we want the
+    # maximum score.
     self.score = -item.score
+
   def clear(self):
     self.item = None
+
   def __bool__(self):
-    return self.item is None
+    return self.item is not None
+
+  # Python 2 uses __nonzero__, while Python 3 uses __bool__.
+  __nonzero__ = __bool__
+
+  # The rest operations are generated by functools.total_ordering decorator.
   def __eq__(self, other):
     return self.score == other.score
+
   def __le__(self, other):
     return self.score <= other.score
 
diff --git a/tools/releasetools/test_blockimgdiff.py b/tools/releasetools/test_blockimgdiff.py
index e5a369478..7084e2153 100644
--- a/tools/releasetools/test_blockimgdiff.py
+++ b/tools/releasetools/test_blockimgdiff.py
@@ -16,12 +16,43 @@
 
 from __future__ import print_function
 
-import common
 import unittest
 
-from blockimgdiff import BlockImageDiff, EmptyImage, Transfer
+import common
+from blockimgdiff import BlockImageDiff, EmptyImage, HeapItem, Transfer
 from rangelib import RangeSet
 
+
+class HealpItemTest(unittest.TestCase):
+
+  class Item(object):
+    def __init__(self, score):
+      self.score = score
+
+  def test_init(self):
+    item1 = HeapItem(self.Item(15))
+    item2 = HeapItem(self.Item(20))
+    item3 = HeapItem(self.Item(15))
+    self.assertTrue(item1)
+    self.assertTrue(item2)
+    self.assertTrue(item3)
+
+    self.assertNotEqual(item1, item2)
+    self.assertEqual(item1, item3)
+    # HeapItem uses negated scores.
+    self.assertGreater(item1, item2)
+    self.assertLessEqual(item1, item3)
+    self.assertTrue(item1 <= item3)
+    self.assertFalse(item2 >= item1)
+
+  def test_clear(self):
+    item = HeapItem(self.Item(15))
+    self.assertTrue(item)
+
+    item.clear()
+    self.assertFalse(item)
+
+
 class BlockImageDiffTest(unittest.TestCase):
 
   def test_GenerateDigraphOrder(self):