<p><a href="https://gerrit.osmocom.org/12556">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c">File tests/trans/trans_test.c:</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/12556/9/tests/trans/trans_test.c@79">Patch Set #9, Line 79:</a> <code style="font-family:monospace,monospace">return</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I don't. I'd rather see as many errors as possible in a single run.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It doesn't make sense to continue execution of the test case if at least one step fails IMHO. Moreover, ran_conn_alloc() may return NULL without any debug messages.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@83">Patch Set #9, Line 83:</a> <code style="font-family:monospace,monospace">return</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">... and here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">trans_alloc() also may return NULL without any debug output.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@88">Patch Set #9, Line 88:</a> <code style="font-family:monospace,monospace">return</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">trans_has_conn() returns pointer to a struct. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">It would explicitly indicate why *and where* the test case has failed. The current code would silently skip the pending code and start testing test_tran_overflow().</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@134">Patch Set #9, Line 134:</a> <code style="font-family:monospace,monospace">base_callref</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sorry, but I am not getting this. The key idea is that allocating multiple transactions with same callref is wrong, and we shouldn't do this at least in tests. Vice versa, this test should ensure that trans_alloc() prevents this.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Changing allocation logic</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">You don't need to change the logic, just do:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  callref = base_callref + i</pre></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12556">change 12556</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/12556"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I78dfb7cd35073a305cf668beda7d9d58d5a5a713 </div>
<div style="display:none"> Gerrit-Change-Number: 12556 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 15 Jan 2019 10:02:14 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>