Change in osmocom-bb[master]: trx_toolkit/data_dump.py: fix return value of parse_msg()

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/18396 )

Change subject: trx_toolkit/data_dump.py: fix return value of parse_msg()
......................................................................

trx_toolkit/data_dump.py: fix return value of parse_msg()

Jenkins build #2516 has uncovered a problem in DATADumpFile.parse_msg():

  ======================================================================
  FAIL: test_parse_empty (test_data_dump.DATADump_Test)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/build/src/target/trx_toolkit/test_data_dump.py",
         line 138, in test_parse_empty
      self.assertEqual(msg, False)
  AssertionError: None != False

I did a quick investigation, and figured out that this failure
happens when trying to call parse_msg() with idx == 0, because
DATADumpFile._seek2msg() basically does nothing in this case
and thus always returns True. The None itself comes from
DATADumpFile._parse_msg().

Let's ensure that DATADumpFile.parse_msg() always returns None,
even if DATADumpFile._seek2msg() fails. Also, update the unit
test, so we always test a wide range of 'idx' values.

Change-Id: Ifcfa9c5208636a0f9309f5ba8e47d282dc6a03f4
---
M src/target/trx_toolkit/data_dump.py
M src/target/trx_toolkit/test_data_dump.py
2 files changed, 6 insertions(+), 6 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved



diff --git a/src/target/trx_toolkit/data_dump.py b/src/target/trx_toolkit/data_dump.py
index 7c5452b..41af695 100644
--- a/src/target/trx_toolkit/data_dump.py
+++ b/src/target/trx_toolkit/data_dump.py
@@ -158,14 +158,14 @@
 	# Parses a particular message defined by index idx
 	# Return value:
 	#   a parsed message in case of success,
-	#   or None in case of EOF or header parsing error,
-	#   or False in case of message parsing error or out of range.
+	#   or None in case of EOF, out of range, or header parsing error,
+	#   or False in case of message parsing error.
 	def parse_msg(self, idx):
 		# Move descriptor to the beginning of requested message
 		rc = self._seek2msg(idx)
 		if not rc:
 			log.error("Couldn't find requested message")
-			return False
+			return None
 
 		# Attempt to parse a message
 		return self._parse_msg()
diff --git a/src/target/trx_toolkit/test_data_dump.py b/src/target/trx_toolkit/test_data_dump.py
index 2f7e25a..5129196 100644
--- a/src/target/trx_toolkit/test_data_dump.py
+++ b/src/target/trx_toolkit/test_data_dump.py
@@ -133,9 +133,9 @@
 
 	def test_parse_empty(self):
 		with self.assertLogs(level = 'ERROR'):
-			idx = random.randrange(100)
-			msg = self._ddf.parse_msg(idx)
-			self.assertEqual(msg, False)
+			for idx in range(100):
+				msg = self._ddf.parse_msg(idx)
+				self.assertEqual(msg, None)
 
 	def test_parse_all_empty(self):
 		msg_list = self._ddf.parse_all()

-- 
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/18396
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Ifcfa9c5208636a0f9309f5ba8e47d282dc6a03f4
Gerrit-Change-Number: 18396
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200522/ca705768/attachment.htm>