<p>Patch set 3:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18471">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18471/3/src/osmo_gsm_tester/obj/ms_srs.py">File src/osmo_gsm_tester/obj/ms_srs.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18471/3/src/osmo_gsm_tester/obj/ms_srs.py@132">Patch Set #3, Line 132:</a> <code style="font-family:monospace,monospace">        MainLoop.sleep(self, self.stop_timeout)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm sorry to tell you that's not the correct approach, since stop() method is only called by tests whilling to stop the object. The usual process tear down is done by testenv from the process registerd in line 170 ( self.testenv.remember_to_stop(self.process)), where the process object is used directly and it will then call cleanup() method in this class.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So the fix is not right. As I said, leave this is as it was and add a boolean flag in stop() or verify_metric(), and sleep in cleanup() only if we didn't already sleep.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18471">change 18471</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/osmo-gsm-tester/+/18471"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-gsm-tester </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I47e46b8ccce41c9a62d2d6866260d22c927e710d </div>
<div style="display:none"> Gerrit-Change-Number: 18471 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: srs_andre <andre@softwareradiosystems.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: srs_andre <andre@softwareradiosystems.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 26 May 2020 10:38:00 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>