Patch Set 3: Code-Review-1 (3 comments)
I'd be in favor of keeping the macro, but adding another parameter to it. https://gerrit.osmocom.org/#/c/6296/1/tests/msc_vlr/msc_vlr_test_rest.c File tests/msc_vlr/msc_vlr_test_rest.c: Line 26: void test_early_stage(const char *imsi) This doesn't seem to belong here https://gerrit.osmocom.org/#/c/6296/1/tests/msc_vlr/msc_vlr_tests.c File tests/msc_vlr/msc_vlr_tests.c: Line 196: accepted = msc_subscr_conn_is_accepted(conn); There are no more asserts in this function which means the test might not reliably fail. Having a macro insert the assert into the actual test function also helps with debugging where exactly a test case failed. https://gerrit.osmocom.org/#/c/6296/1/tests/msc_vlr/msc_vlr_tests.h File tests/msc_vlr/msc_vlr_tests.h: Line 147 Instead of converting the macro to a function you can just add a parameter for g_conn here -- To view, visit https://gerrit.osmocom.org/6296 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I36ae1f9bb395921dc2c5a39e35fbb8040ba47213 Gerrit-PatchSet: 3 Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Owner: Max <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-HasComments: Yes
