| Message ID | 1437488613-3943-18-git-send-email-pablo@gnumonks.org |
|---|---|
| State | Accepted |
| Headers | show |
> On 21 Jul 2015, at 16:23, pablo@gnumonks.org wrote: > > -AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) > +AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g I like -ggdb3 :) In terms of the tests. You run the code but I don’t see many asserts. Could you please explain how the “no voice traffic” code tests? Would a lack of data end printing a message that would cause the test to fail? holger
> On 10 Aug 2015, at 10:33, Pablo Neira Ayuso <pablo@gnumonks.org> wrote: > > On Sat, Aug 08, 2015 at 09:11:39PM +0200, Holger Freyther wrote: >> >>> On 21 Jul 2015, at 16:23, pablo@gnumonks.org wrote: >>> >>> -AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) >>> +AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g >> >> I like -ggdb3 :) > > OK, will change this. This was just personal taste. No need to update if -g was good enough for you. I tend to compile with CFLAGS=“-Og -ggdb3” make > Fine with this procedure? Let me know if I'm missing anything. yes, that is fine.
On Sat, Aug 08, 2015 at 09:11:39PM +0200, Holger Freyther wrote: > > > On 21 Jul 2015, at 16:23, pablo@gnumonks.org wrote: > > > > -AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) > > +AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g > > I like -ggdb3 :) OK, will change this. > In terms of the tests. You run the code but I don’t see many asserts. Could you > please explain how the “no voice traffic” code tests? Would a lack of data end > printing a message that would cause the test to fail? The test most likely will timeout if it doesn't get any voice traffic. But I think it's a good idea to add more asserts. In summary: 1) Will rename dummy to ndummy. 2) Will mangle this patch to replace -g to -ggdb3. And I will push out this patchset, then will send two follow up patches: 3) Will revisit the one timer running + circuit gone scenario. This will come in a follow up patch since this problem since to be there since the beginning. 4) Will add more asserts to validate osmux outputs from tests. Fine with this procedure? Let me know if I'm missing anything. Thanks for reviewing.
On Mon, Aug 10, 2015 at 10:29:21AM +0200, Holger Freyther wrote: [...] > > Fine with this procedure? Let me know if I'm missing anything. > > yes, that is fine. Just pushed out this to master, including a patch to rename dummy to ndummy as you suggested. Will follow up soon by reviewing the other concerns you spot during review. Cheers from Seattle!
> On 19 Aug 2015, at 02:52, Pablo Neira Ayuso <pablo@gnumonks.org> wrote: > Hi! > Just pushed out this to master, including a patch to rename dummy to > ndummy as you suggested. Will follow up soon by reviewing the other > concerns you spot during review. the CI fails because of missing documentation strings in the new command Documentation error (missing docs): <command id='osmux dummy (on|off)'> <param name='off' doc='(null)' /> Documentation error (missing docs): <command id='osmux dummy (on|off)'> <param name='off' doc='(null)' /> with the VTY code every parameter needs to be documented in this case it needs to be: osmux dummy on off Given the error message I think everything but off is documented. enjoy the city! holger
diff --git a/tests/Makefile.am b/tests/Makefile.am index a986940..f99e276 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,4 +1,4 @@ -AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) +AM_CFLAGS = -Wall -I$(top_srcdir)/include $(LIBOSMOCORE_CFLAGS) -g AM_LDFLAGS = $(LIBOSMOCORE_LDFLAGS) check_PROGRAMS = osmux/osmux_test
From: Pablo Neira Ayuso <pablo@soleta.eu> Useful when debuggin via valgrind/gdb. --- tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)