In-Reply-To: <8a018620-25a7-a292-c951-dd2017d54...@redhat.com>
On Mon, May 03, 2021 at 12:53:39PM +0200, Petr Menšík wrote:
> On 4/30/21 12:42 AM, Simon Kelley wrote:
> > On 14/04/2021 18:35, Petr Menšík wrote:
> >> Hi Simon and other dnsmasq friends,
> >>
> >> after some struggling with Makefile support, I am sending my dnsmasq
> >> unit tests. It uses another directory with tests specific code. I moved
> >> some common parts to Makefile.config, in order to be able to reuse them.
> >> Unit tests are under tests directory with own Makefile.
> >>
> >> New target make check should work also from top directory. Some checks
> >> would work only from tests directory (make kyua). Current coverage is
> >> rather poor, but I hope can be used as a building block to better tests.
> >> Especially option parsing tests are easy to write. Testing of sending
> >> and receiving packets seems to be difficult, it should be tested by
> >> different kind of test IMHO.
> >>
> >> First is attempt to refactor, the second is what evolved into more
> >> complex set of tests.
> >>
> >> Original separate commits are still available on github [1].
> >>
> >> What do you think?
> >
> > Well, I applied the patch, and run "make check" and all the tests passed!
> >
> > Now I have to understand how to write new tests.
>
> Configuration parsing tests are easy, just provide input parameters
> similar way to existing test and then check expected values are provided.
> >
> > Would it make sense to consider some changes to the main code to make
> > the tests easier? I see that die() is a problem. Can we change the code
> > in die() to do something useful when testing?
>
> I have chosen to omit dnsmasq.c code from tests. It contains main()
> function, cannot be part of test anyway. Sure, some code changes would
> help with reducing needed repetitions in tests. Especially init code
> required in tests should be moved out of dnsmasq.c, where it could be
> called directly from tests. Shared init code must not be static
> functions of course.
>
> die does make sense everywhere where it is a corner case. If we move
> die() calls to dnsmasq.c, it would be okay. Other files should return
> indication of fatal error, but not die directly. It would need
> additional wrappers in dnsmasq.c, but such functions would be more testable.
> >
> > Also the tests seem to can copies of initialisation code, does it make
> > sense to abstract the initialisation in main() so that it can be used by
> > the tests standalone?
> Yes, it make sense to move parts of initialization to subsystem-specific
> initialization functions. I would move dns_init() into rfc1035.c,
> dhcp_init() into dhcp-common.c etc. It should make main source file
> shorter and it would be more obvious, which subsystems are initialized
> in which order, whether they depend on anything before it. I think the
> best practice is to break long functions into several shorter, more
> readable functions. I think current main() is a great example to break
> into more smaller functions and move some of them to shareable files.
> Parts required by current tests are small enough.
> >
> > I'm thinking of changing the existing main()
> >
> > main()
> > {
> > <initialise stuff>
> > while (1)
> > events()
> > }
> >
> > into
> >
> > main()
> > {
> > init();
> > while (1)
> > events()
> > }
> >
> > So that init() is available for testing.
> >
> >
> > Cheers,
> >
> > Simon.
> >
> >>
> >> PS: sending this message again, because patch #2 were big enough to
> >> require moderator's approval. Compressed it as a workaround.
> >>
> >> Cheers,
> >> Petr
> >>
> >> 1. https://github.com/InfrastructureServices/dnsmasq/tree/unittests
> What was / is the posting from Simon asking something Would unittest have detect this side-effect of the change? I couldn't find it, but could find the above posting. Reason for starting a fresh thread is for having fresh attention for unittests. Groeten Geert Stappers -- Silence is hard to parse
signature.asc
Description: PGP signature
_______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
