Code review comment for lp:~midauth-pisa-devs/hipl/midauth-firewall

On Wed, Oct 26, 2011 at 11:16:18PM +0200, Christof Mroz wrote:
> On 26.10.2011 22:57, Diego Biurrun wrote:
> >On Tue, Oct 25, 2011 at 11:43:43PM +0200, Christof Mroz wrote:
> >>On 25.10.2011 23:02, Diego Biurrun wrote:
> >>>On Tue, Oct 25, 2011 at 02:35:31PM +0000, René Hummen wrote:
> >>>>On 25.10.2011, at 15:24, Diego Biurrun wrote:
> >>>>>On Tue, Oct 25, 2011 at 02:28:06PM +0200, René Hummen wrote:
> >>>>>>On 20.10.2011, at 14:15, Diego Biurrun wrote:
> >>>>>>>On Wed, Oct 19, 2011 at 01:17:30PM +0000, René Hummen wrote:
> >>>>>>>
> >>>>>>>>--- Makefile.am 2011-10-17 18:14:10 +0000
> >>>>>>>>+++ Makefile.am 2011-10-19 13:13:48 +0000
> >>>>>>>>@@ -253,7 +256,7 @@
> >>>>>>>>
> >>>>>>>>-test_check_firewall_LDFLAGS = -ldl
> >>>>>>>>+test_check_firewall_LDFLAGS = -ldl -z muldefs
> >>>>>>>
> >>>>>>>We have solved this differently elsewhere by #including the .c file to
> >>>>>>>unit-test static functions.
> >>>>>>
> >>>>>>This has been discussed previously on the list:
> >>>>>> http://www.freelists.org/post/hipl-dev/Mock-functions-for-hipl-unit-tests
> >>>>>
> >>>>>But we currently don't favor that solution. If that needs changing,
> >>>>>it's a topic for a separate merge request and would need to be changed
> >>>>>in all places.
> >>>>
> >>>>The change was necessary in order to comply with our unit-test for new
> >>>>code policy as unit-tests would otherwise not link correctly. The only
> >>>>other option I see right now is to merge the code without unit-tests.
> >>>
> >>>No, the change is not necessary. As I said before, we have solved
> >>>this differently elsewhere by #including the .c file to unit-test
> >>>static functions.
> >>
> >>Including the .c file allows you to access static identifiers in
> >>unit tests, but the libipq problem (see referenced thread) is a
> >>different one.
> >>Apparently, Hendrik already tried to mimic the other (working) tests
> >>already without success.
> >
> >I'd like to know what is failing exactly and how before making
> >a judgement here.
>
> See the referenced thread for details, but essentially we need to
> mock ipq_get_packet() which is usually provided by linking -lipq.

What I don't get is the reason why we need to mock that function
in the first place. Probably I am missing something totally
obvious.

Diego