Max has posted comments on this change. ( https://gerrit.osmocom.org/12556 )

Change subject: Add tests for transaction routines
......................................................................


Patch Set 10:

(1 comment)

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@134
PS9, Line 134: base_callref
> The key idea is that allocating multiple transactions with same callref is 
> wrong, and we shouldn't do this at least in tests.

I strongly disagree. We should never try to "fix" issues by making tests behave 
differently from the code. The right way is to add test which illustrate 
current code behavior (even if it's wrong), than fix the code and update the 
test accordingly.



--
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: 10
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Vadim Yanitskiy <[email protected]>
Gerrit-Comment-Date: Tue, 15 Jan 2019 10:36:10 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to