Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12556 )
Change subject: Add tests for transaction routines ...................................................................... Patch Set 9: (4 comments) https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c File tests/trans/trans_test.c: https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@79 PS9, Line 79: return > I don't. I'd rather see as many errors as possible in a single run. 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. https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@83 PS9, Line 83: return > ... and here. trans_alloc() also may return NULL without any debug output. https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@88 PS9, Line 88: return > trans_has_conn() returns pointer to a struct. […] 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(). https://gerrit.osmocom.org/#/c/12556/9/tests/trans/trans_test.c@134 PS9, Line 134: base_callref 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. > Changing allocation logic You don't need to change the logic, just do: callref = base_callref + i -- To view, visit https://gerrit.osmocom.org/12556 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78dfb7cd35073a305cf668beda7d9d58d5a5a713 Gerrit-Change-Number: 12556 Gerrit-PatchSet: 9 Gerrit-Owner: Max <msur...@sysmocom.de> Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <msur...@sysmocom.de> Gerrit-Reviewer: Vadim Yanitskiy <axilira...@gmail.com> Gerrit-Comment-Date: Tue, 15 Jan 2019 10:02:14 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
