Re: [lwip-devel] (unit) testing of lwip

[Mate Varga]

Hi,

How should I approach DNS-related testing? The specific change I'd like to make 
(feedback welcome): in dns.c, the dns_send() function splits the hostname into 
multiple parts and populates the packet buffer's QNAME field. This has/had a 
bug that is trivial to fix (see patch on Savannah), but in order to test it, I 
need to extract this functionality into a separate function (that can be tested 
in isolation), something like 

/**
 * Convert hostname into suitable query format
 * and populate QNAME region of the packet buffer
 *
 * @param hostname the target host's name
 * @param pre-allocated p buffer for the outbound UDP packet
 * @return position of the end of the QNAME region
 */
err_t
dns_convert_hostname(const char * hostname, struct pbuf *p, u16_t *query_idx);

My problem is that the the default test suite has LWIP_DNS undefined (well, 
def'd to 0), therefore dns.c is not included in the build.

What do you recommend? 
1) compile the DNS code when running unit tests (define LWIP_DNS and iron out 
issues like that pbuf and mem unit tests require LWIP_DNS to be falsy)
2) move this utility method out from dns.c 
3) something else

Thanks,
Mate

On Mon, Jun 1, 2020, at 11:51, Mate Varga wrote:
> Ah okay, thanks. I have probably read some old documentation about who 
> to run the test suite. I'll submit the other changeset then soon.
> 
> On Mon, Jun 1, 2020, at 11:45, goldsimon wrote:
> > 
> > 
> > Am 1. Juni 2020 11:37:28 MESZ schrieb Mate Varga <m@matevarga.net>:
> > >Hello,
> > >
> > >I have made a small bug fix for lwip (#58473), and as a follow-up I'd
> > >like to make sure that this simple, compact functionality (converting
> > >hostname into a QNAME field) is unit tested. I have written a test and
> > >I'm trying to run the test suite, but it looks to me that the suite is
> > >broken in various ways (building it under *nix + gcc doesn't work, I
> > >can get into details later about how exactly).
> > 
> > It works for me...
> > 
> > >
> > >I can try and fix the tests, but first I'd like to understand whether
> > >it is the intention of the project owners to keep the suite and the
> > >examples maintained -- I don't want to submit a patch and find out that
> > >it's not welcome :). Also it looks a bit odd that lwip's unit tests are
> > >run from and depend on lwip_contrib.
> > 
> > There's no "lwip_contrib" any more. The files are integrated into the 
> > mail repo now. I guess you're not using latest git master?
> > 
> > Regards,
> > Simon
> > 
> > >
> > >Should I invest effort into making some refactoring here? First of all
> > >I'd probably try to run unit tests without requiring lwip_contrib.
> > >
> > >Thanks,
> > >Mate
> > 
> > _______________________________________________
> > lwip-devel mailing list
> > lwip-devel@nongnu.org
> > https://lists.nongnu.org/mailman/listinfo/lwip-devel
> >
> 
> _______________________________________________
> lwip-devel mailing list
> lwip-devel@nongnu.org
> https://lists.nongnu.org/mailman/listinfo/lwip-devel
>