<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmocom-bb/+/18396">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">trx_toolkit/data_dump.py: fix return value of parse_msg()<br><br>Jenkins build #2516 has uncovered a problem in DATADumpFile.parse_msg():<br><br> ======================================================================<br> FAIL: test_parse_empty (test_data_dump.DATADump_Test)<br> ----------------------------------------------------------------------<br> Traceback (most recent call last):<br> File "/build/src/target/trx_toolkit/test_data_dump.py",<br> line 138, in test_parse_empty<br> self.assertEqual(msg, False)<br> AssertionError: None != False<br><br>I did a quick investigation, and figured out that this failure<br>happens when trying to call parse_msg() with idx == 0, because<br>DATADumpFile._seek2msg() basically does nothing in this case<br>and thus always returns True. The None itself comes from<br>DATADumpFile._parse_msg().<br><br>Let's ensure that DATADumpFile.parse_msg() always returns None,<br>even if DATADumpFile._seek2msg() fails. Also, update the unit<br>test, so we always test a wide range of 'idx' values.<br><br>Change-Id: Ifcfa9c5208636a0f9309f5ba8e47d282dc6a03f4<br>---<br>M src/target/trx_toolkit/data_dump.py<br>M src/target/trx_toolkit/test_data_dump.py<br>2 files changed, 6 insertions(+), 6 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/target/trx_toolkit/data_dump.py b/src/target/trx_toolkit/data_dump.py</span><br><span>index 7c5452b..41af695 100644</span><br><span>--- a/src/target/trx_toolkit/data_dump.py</span><br><span>+++ b/src/target/trx_toolkit/data_dump.py</span><br><span>@@ -158,14 +158,14 @@</span><br><span> # Parses a particular message defined by index idx</span><br><span> # Return value:</span><br><span> # a parsed message in case of success,</span><br><span style="color: hsl(0, 100%, 40%);">- # or None in case of EOF or header parsing error,</span><br><span style="color: hsl(0, 100%, 40%);">- # or False in case of message parsing error or out of range.</span><br><span style="color: hsl(120, 100%, 40%);">+ # or None in case of EOF, out of range, or header parsing error,</span><br><span style="color: hsl(120, 100%, 40%);">+ # or False in case of message parsing error.</span><br><span> def parse_msg(self, idx):</span><br><span> # Move descriptor to the beginning of requested message</span><br><span> rc = self._seek2msg(idx)</span><br><span> if not rc:</span><br><span> log.error("Couldn't find requested message")</span><br><span style="color: hsl(0, 100%, 40%);">- return False</span><br><span style="color: hsl(120, 100%, 40%);">+ return None</span><br><span> </span><br><span> # Attempt to parse a message</span><br><span> return self._parse_msg()</span><br><span>diff --git a/src/target/trx_toolkit/test_data_dump.py b/src/target/trx_toolkit/test_data_dump.py</span><br><span>index 2f7e25a..5129196 100644</span><br><span>--- a/src/target/trx_toolkit/test_data_dump.py</span><br><span>+++ b/src/target/trx_toolkit/test_data_dump.py</span><br><span>@@ -133,9 +133,9 @@</span><br><span> </span><br><span> def test_parse_empty(self):</span><br><span> with self.assertLogs(level = 'ERROR'):</span><br><span style="color: hsl(0, 100%, 40%);">- idx = random.randrange(100)</span><br><span style="color: hsl(0, 100%, 40%);">- msg = self._ddf.parse_msg(idx)</span><br><span style="color: hsl(0, 100%, 40%);">- self.assertEqual(msg, False)</span><br><span style="color: hsl(120, 100%, 40%);">+ for idx in range(100):</span><br><span style="color: hsl(120, 100%, 40%);">+ msg = self._ddf.parse_msg(idx)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.assertEqual(msg, None)</span><br><span> </span><br><span> def test_parse_all_empty(self):</span><br><span> msg_list = self._ddf.parse_all()</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmocom-bb/+/18396">change 18396</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmocom-bb/+/18396"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmocom-bb </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ifcfa9c5208636a0f9309f5ba8e47d282dc6a03f4 </div>
<div style="display:none"> Gerrit-Change-Number: 18396 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>