You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jacob Champion <ch...@gmail.com> on 2017/05/25 21:44:49 UTC

Experimental C unit test suite available for hacking

Hi all,

Last week I had a personal hackathon since I couldn't make it out to 
ApacheCon. As a result there is now a C-language unit test suite 
available in branches/httpdunit (based on trunk). I've tested it with a 
Windows+CMake toolchain as well as an Ubuntu+autoconf toolchain.

The suite itself is based on Check, which is a testing library I've had 
some success with in the past. It's supported on a wide variety of 
platforms and has a nice feature of running each test in a separate 
process space, so a crash doesn't derail the entire suite. (Note: Check 
is LGPL.) The build system has been augmented slightly to generate some 
of the more tedious boilerplate code.

If you want to give it a try, just install Check (and, if using the 
configure scripts, make sure Check is visible via pkg-config). The test 
suite will then automatically be added to the default targets. Once 
everything builds you just run the suite directly with

     $ ./test/httpdunit

As a Check binary, it has multiple knobs to control which tests run and 
how the reporting is done, but by default it just runs all the tests and 
prints TAP to stdout.

The example tests that are currently running are testing a new API for 
strict Base64 decoding. Right now it's a feature without a client; I 
included it here because it was a good showcase of the test suite (see 
test/unit/base64.c for the test case code).

Let me know what you think!

--Jacob

Re: Experimental C unit test suite available for hacking

Posted by Jacob Champion <ch...@gmail.com>.
On Wed, May 23, 2018, 7:35 AM Micha Lenk <mi...@lenk.info> wrote:

> > It should only get built if you configure --with-test-suite=... and
> > specify the path to a perl-framework wc.  It builds for me in trunk.
>
> Hmm, no difference. It seems to get built independent from whether you
> specify --with-test-suite or not. As a cross check I've just added
> "--with-test-suite=../httpd-test" to the configure flags, but I still
> get the same build failure.
>

Hi Micha, thanks for your interest! I won't be able to debug into this
until later in the day, but I'll try to take a look.

You asked about buildbot -- IIRC, several pieces of the test suite don't
run there, but it's been months since I last looked.

--Jacob

>

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Joe,

On 05/23/2018 04:21 PM, Joe Orton wrote:
> On Wed, May 23, 2018 at 04:14:39PM +0200, Micha Lenk wrote:
>> Hi Eric,
>>
>> On 05/23/2018 02:59 PM, Eric Covener wrote:
>>> I guess the CI setup needs to be updated to at least build the unit tests?
>>
>> I did not configure the build explicitly to run the unit tests, so it is
>> just the plain "make" that causes this target to get build. I would expect
>> the CI setup to implicitly build it as well. Yes, no?!
>>
>> Does the target 'test/httpdunit' not get build in your local builds?
> 
> It should only get built if you configure --with-test-suite=... and
> specify the path to a perl-framework wc.  It builds for me in trunk.

Hmm, no difference. It seems to get built independent from whether you 
specify --with-test-suite or not. As a cross check I've just added 
"--with-test-suite=../httpd-test" to the configure flags, but I still 
get the same build failure.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Marion et Christophe JAILLET <ch...@wanadoo.fr>.

Le 23/05/2018 à 16:21, Joe Orton a écrit :
> On Wed, May 23, 2018 at 04:14:39PM +0200, Micha Lenk wrote:
>> Hi Eric,
>>
>> On 05/23/2018 02:59 PM, Eric Covener wrote:
>>> I guess the CI setup needs to be updated to at least build the unit tests?
>> I did not configure the build explicitly to run the unit tests, so it is
>> just the plain "make" that causes this target to get build. I would expect
>> the CI setup to implicitly build it as well. Yes, no?!
>>
>> Does the target 'test/httpdunit' not get build in your local builds?
> It should only get built if you configure --with-test-suite=... and
> specify the path to a perl-framework wc.  It builds for me in trunk.
>
> Regards, Joe
>
Hi,

'--with-test-suite=...' is for the Perl test framework, not for the 'test/httpdunit'.
'test/httpdunit' is designed for unit tests, written in C, and is included in trunk.

Up to now, there is not that much in it, but at least a framework is available.

CJ



Re: Experimental C unit test suite available for hacking

Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 23, 2018 at 04:14:39PM +0200, Micha Lenk wrote:
> Hi Eric,
> 
> On 05/23/2018 02:59 PM, Eric Covener wrote:
> > I guess the CI setup needs to be updated to at least build the unit tests?
> 
> I did not configure the build explicitly to run the unit tests, so it is
> just the plain "make" that causes this target to get build. I would expect
> the CI setup to implicitly build it as well. Yes, no?!
> 
> Does the target 'test/httpdunit' not get build in your local builds?

It should only get built if you configure --with-test-suite=... and 
specify the path to a perl-framework wc.  It builds for me in trunk.

Regards, Joe

Re: Experimental C unit test suite available for hacking

Posted by Marion et Christophe JAILLET <ch...@wanadoo.fr>.

Le 23/05/2018 à 16:14, Micha Lenk a écrit :
> Hi Eric,
>
> On 05/23/2018 02:59 PM, Eric Covener wrote:
>> I guess the CI setup needs to be updated to at least build the unit 
>> tests?
>
> I did not configure the build explicitly to run the unit tests, so it 
> is just the plain "make" that causes this target to get build. I would 
> expect the CI setup to implicitly build it as well. Yes, no?!
>
> Does the target 'test/httpdunit' not get build in your local builds?
>
> Cheers,
> Micha Lenk
>
'test/httpdunit' is always built in my dev machine.

This is added automatically to the 'other_target' if the 'check' library 
(>= version 0.9.12) is found on the system at configuration time.
CJ

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Eric,

On 05/23/2018 02:59 PM, Eric Covener wrote:
> I guess the CI setup needs to be updated to at least build the unit tests?

I did not configure the build explicitly to run the unit tests, so it is 
just the plain "make" that causes this target to get build. I would 
expect the CI setup to implicitly build it as well. Yes, no?!

Does the target 'test/httpdunit' not get build in your local builds?

Cheers,
Micha Lenk

Re: Experimental C unit test suite available for hacking

Posted by Eric Covener <co...@gmail.com>.
On Wed, May 23, 2018 at 8:48 AM, Micha Lenk <mi...@lenk.info> wrote:
> On 05/25/2017 11:44 PM, Jacob Champion wrote (almost a year ago):
>>
>> Last week I had a personal hackathon since I couldn't make it out to
>> ApacheCon. As a result there is now a C-language unit test suite available
>> in branches/httpdunit (based on trunk). I've tested it with a Windows+CMake
>> toolchain as well as an Ubuntu+autoconf toolchain.
>>
>> The suite itself is based on Check, which is a testing library I've had
>> some success with in the past. It's supported on a wide variety of platforms
>> and has a nice feature of running each test in a separate process space, so
>> a crash doesn't derail the entire suite. (Note: Check is LGPL.) The build
>> system has been augmented slightly to generate some of the more tedious
>> boilerplate code.
>>
>> If you want to give it a try, just install Check (and, if using the
>> configure scripts, make sure Check is visible via pkg-config). The test
>> suite will then automatically be added to the default targets. Once
>> everything builds you just run the suite directly with
>>
>>      $ ./test/httpdunit
>>
>> As a Check binary, it has multiple knobs to control which tests run and
>> how the reporting is done, but by default it just runs all the tests and
>> prints TAP to stdout.
>>
>> The example tests that are currently running are testing a new API for
>> strict Base64 decoding. Right now it's a feature without a client; I
>> included it here because it was a good showcase of the test suite (see
>> test/unit/base64.c for the test case code).
>>
>> Let me know what you think!
>
>
> I totally like the idea of having unit tests for httpd, so thanks a lot for
> adding them!
>
> I recently struggled compiling trunk locally on Debian 9.4 because of these
> unit tests. Apparently it fails to link test/httpdunit because of undefined
> references to ap_queue_info_push_pool and other symbols. See the attached
> build log for more details.
>
> Does anybody have an idea what went wrong? Am I missing something?
> How does buildbot build?

I think there was a subsequent change to factor some of the shared
queue stuff out of event/worker, probably missing just a few
additional targets now.

I guess the CI setup needs to be updated to at least build the unit tests?

Re: Experimental C unit test suite available for hacking

Posted by Steffen <in...@apachelounge.com>.
Building on Windows it errors.

branches/httpdunit is a based on an "old" trunk which does not build.

Current trunk build on windows.

On 23-05-18 14:48, Micha Lenk wrote:
> On 05/25/2017 11:44 PM, Jacob Champion wrote (almost a year ago):
>> Last week I had a personal hackathon since I couldn't make it out to 
>> ApacheCon. As a result there is now a C-language unit test suite 
>> available in branches/httpdunit (based on trunk). I've tested it with 
>> a Windows+CMake toolchain as well as an Ubuntu+autoconf toolchain.
>>
>> The suite itself is based on Check, which is a testing library I've 
>> had some success with in the past. It's supported on a wide variety 
>> of platforms and has a nice feature of running each test in a 
>> separate process space, so a crash doesn't derail the entire suite. 
>> (Note: Check is LGPL.) The build system has been augmented slightly 
>> to generate some of the more tedious boilerplate code.
>>
>> If you want to give it a try, just install Check (and, if using the 
>> configure scripts, make sure Check is visible via pkg-config). The 
>> test suite will then automatically be added to the default targets. 
>> Once everything builds you just run the suite directly with
>>
>>      $ ./test/httpdunit
>>
>> As a Check binary, it has multiple knobs to control which tests run 
>> and how the reporting is done, but by default it just runs all the 
>> tests and prints TAP to stdout.
>>
>> The example tests that are currently running are testing a new API 
>> for strict Base64 decoding. Right now it's a feature without a 
>> client; I included it here because it was a good showcase of the test 
>> suite (see test/unit/base64.c for the test case code).
>>
>> Let me know what you think!
>
> I totally like the idea of having unit tests for httpd, so thanks a 
> lot for adding them!
>
> I recently struggled compiling trunk locally on Debian 9.4 because of 
> these unit tests. Apparently it fails to link test/httpdunit because 
> of undefined references to ap_queue_info_push_pool and other symbols. 
> See the attached build log for more details.
>
> Does anybody have an idea what went wrong? Am I missing something?
> How does buildbot build?
>
> Best regards,
> Micha


Re: Experimental C unit test suite available for hacking

Posted by Jacob Champion <ch...@gmail.com>.
On 05/23/2018 10:32 AM, Marion et Christophe JAILLET wrote:
 > '--with-test-suite=...' is for the Perl test framework, not for the
 > 'test/httpdunit'.

Ah, yes! Thanks for the reminder; --with-test-suite only enables the 
`make check` functionality. Sorry, it's been too long.

On 05/23/2018 01:21 PM, Christophe Jaillet wrote:
> I can reproduce the issue if I don't pass any --enable-mpms-shared 
> paramater to ./configure.

Me too. The problem, I think, is that the $(MPM_LIB) depends on symbols 
in server/libmain.la, but comes after it in the $(PROGRAM_DEPENDENCIES). 
That won't work. Moving $(MPM_LIB) up above server/libmain.la in the 
top-level Makefile.in seems to solve the problem for me, but I'm not 
able to sufficiently test tonight to make sure it doesn't break 
something else. HTH?

--Jacob

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Yann,

On 05/24/2018 10:41 AM, Yann Ylavic wrote:
> --- Makefile.in	(revision 1832123)
> +++ Makefile.in	(working copy)
> @@ -7,9 +7,9 @@ PROGRAM_SOURCES      = modules.c
>   PROGRAM_LDADD        = buildmark.o $(HTTPD_LDFLAGS) $(PROGRAM_DEPENDENCIES) $(HTTPD_LIBS) $(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
>   PROGRAM_PRELINK      = $(COMPILE) -c $(top_srcdir)/server/buildmark.c
>   PROGRAM_DEPENDENCIES = \
> +  $(MPM_LIB) \
>     server/libmain.la \
>     $(BUILTIN_LIBS) \
> -  $(MPM_LIB) \
>     os/$(OS_DIR)/libos.la
>   
>   sbin_PROGRAMS   = $(PROGRAM_NAME)

I've just tested several different library orders, but none of them work 
for me (even without the problematic modules I mentioned in the other 
mail). The library order as you are suggesting above is the only one 
that works for me.

I'll look into the modules issue later.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 8:11 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> On Thu, May 24, 2018, 06:34 Eric Covener <co...@gmail.com> wrote:
>>
>> Despite the directory structure, this was not part of a "module" in
>> the httpd/LoadModule sense.  I think it's reasonable to pull it "up"
>> which is simpler then trying to push more stuff down into
>> modules/http/.
>
> [Caviet]
> Note that relocation is a major mmn bump, due to two level namespaces...
> Which isn't usual apparent on flat namespace architectures such as Linux.

So finally I did "svn move modules/http/http_etag.c
server/util_etag.c" and adjusted build files in r1833083.
There is no need for backport (for now), no httpdunit tests in 2.4.x,
it wouldn't be possible as is anyway due to MMN major bump.
I tried to keep ap_{make,set}_etag() functions in some
"modules/http/http_etag_glue.c" file, calling some
ap_{make,set}_etag_core() functions in "server/util_etag.c", but
failed. It either didn't build with --enable-mods-shared=reallyall
(export.o can't find ap_{make,set}_etag() for linking), or with
--enable-mods-static=all (httpdunit, with required
PROGRAM_DEPENDENCIES order for "shared" to work)...

Re: Experimental C unit test suite available for hacking

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, May 31, 2018 at 10:07 AM, Yann Ylavic <yl...@gmail.com> wrote:

> On Thu, May 24, 2018 at 8:11 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >
> > On Thu, May 24, 2018, 06:34 Eric Covener <co...@gmail.com> wrote:
> >>
> >> Despite the directory structure, this was not part of a "module" in
> >> the httpd/LoadModule sense.  I think it's reasonable to pull it "up"
> >> which is simpler then trying to push more stuff down into
> >> modules/http/.
> >
> > [Caviet]
> > Note that relocation is a major mmn bump, due to two level namespaces...
> > Which isn't usual apparent on flat namespace architectures such as Linux.
>
> How about:
> 1. we move+rename the functions to "server/protocol.c" and have and
> the "modules/http/http_etag.c" ones simply call the formers (MINOR
> bump).
> 2. we then "svn remove" modules/http/http_etag.c and rename back
> ap_make_etag_core() and ap_set_etag_core() to their original names
> (MAJOR bump, both for namespace and removal of transient _core()
> functions).
> ?
>
> That way 1. would be backportable to 2.4.x, and after 2/ trunk in the
> state we want it to be.
> Does it sound good?
>

That's entirely workable. We just can't change the exports by-module
during the 2.4.* cycle, and the proposal above accomplishes this (but
does make backports more of a headache during that transition.)
As long as the users' code compiles and links against 2.5.* you have
not broken anything, so long as they don't need to change #include's.

It would be nice to have a schematic of where stuff belongs. http2_*
family modules should obviously have those transport-only semantics.
Perhaps evolve transport-only HTTP 1.x http_* semantics into an
http1_* module, for easy discard.

We could keep mod_http(_*) for all HTTP/*.* protocol encoding, but
seeing as this is the httpd server, putting any protocol encoding logic
into core (perhaps later into a libhttpd, e.g. http_etag -> util_etag)
seems rational.

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 8:11 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> On Thu, May 24, 2018, 06:34 Eric Covener <co...@gmail.com> wrote:
>>
>> Despite the directory structure, this was not part of a "module" in
>> the httpd/LoadModule sense.  I think it's reasonable to pull it "up"
>> which is simpler then trying to push more stuff down into
>> modules/http/.
>
> [Caviet]
> Note that relocation is a major mmn bump, due to two level namespaces...
> Which isn't usual apparent on flat namespace architectures such as Linux.

How about:
1. we move+rename the functions to "server/protocol.c" and have and
the "modules/http/http_etag.c" ones simply call the formers (MINOR
bump).
2. we then "svn remove" modules/http/http_etag.c and rename back
ap_make_etag_core() and ap_set_etag_core() to their original names
(MAJOR bump, both for namespace and removal of transient _core()
functions).
?

That way 1. would be backportable to 2.4.x, and after 2/ trunk in the
state we want it to be.
Does it sound good?

Re: Experimental C unit test suite available for hacking

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, May 24, 2018, 06:34 Eric Covener <co...@gmail.com> wrote:

> On Thu, May 24, 2018 at 7:23 AM, Micha Lenk <mi...@lenk.info> wrote:
> > Hi Yann,
> >
> > On 05/24/2018 12:31 PM, Yann Ylavic wrote:
> >>>
> >>> Well, first things first. Let's first fix trunk to be buildable again
> on
> >>> build systems that really only link the needed symbols and thus rely on
> >>> the
> >>> correct library order during linking.
> >>
> >>
> >> I think this is*the*  dependency issue, the order in
> >> PROGRAM_DEPENDENCIES should make modules depend on core and not the
> >> other way around.
> >
> >
> > Yes, sounds reasonable. With the little knowledge about the build system
> I
> > was just blindly moving library orders around without looking into the
> > semantical meaning of these dependencies.
> >
> >> With this patch, both static and shared builds work for me:
> >
> >
> > Confirmed. Works for me too.
> >
> > The only thing that I am a bit concerned about is, why is the HTTP ETag
> > functionality now part of the core, and not part of the http module
> anymore?
> > Isn't this somewhat adverse to other efforts to move more code from core
> to
> > modules?
>
> Despite the directory structure, this was not part of a "module" in
> the httpd/LoadModule sense.  I think it's reasonable to pull it "up"
> which is simpler then trying to push more stuff down into
> modules/http/.
>

[Caviet]
Note that relocation is a major mmn bump, due to two level namespaces...
Which isn't usual apparent on flat namespace architectures such as Linux.

>

Re: Experimental C unit test suite available for hacking

Posted by Eric Covener <co...@gmail.com>.
On Thu, May 24, 2018 at 7:23 AM, Micha Lenk <mi...@lenk.info> wrote:
> Hi Yann,
>
> On 05/24/2018 12:31 PM, Yann Ylavic wrote:
>>>
>>> Well, first things first. Let's first fix trunk to be buildable again on
>>> build systems that really only link the needed symbols and thus rely on
>>> the
>>> correct library order during linking.
>>
>>
>> I think this is*the*  dependency issue, the order in
>> PROGRAM_DEPENDENCIES should make modules depend on core and not the
>> other way around.
>
>
> Yes, sounds reasonable. With the little knowledge about the build system I
> was just blindly moving library orders around without looking into the
> semantical meaning of these dependencies.
>
>> With this patch, both static and shared builds work for me:
>
>
> Confirmed. Works for me too.
>
> The only thing that I am a bit concerned about is, why is the HTTP ETag
> functionality now part of the core, and not part of the http module anymore?
> Isn't this somewhat adverse to other efforts to move more code from core to
> modules?

Despite the directory structure, this was not part of a "module" in
the httpd/LoadModule sense.  I think it's reasonable to pull it "up"
which is simpler then trying to push more stuff down into
modules/http/.

-- 
Eric Covener
covener@gmail.com

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Yann,

On 05/24/2018 12:31 PM, Yann Ylavic wrote:
>> Well, first things first. Let's first fix trunk to be buildable again on
>> build systems that really only link the needed symbols and thus rely on the
>> correct library order during linking.
>
> I think this is*the*  dependency issue, the order in
> PROGRAM_DEPENDENCIES should make modules depend on core and not the
> other way around.

Yes, sounds reasonable. With the little knowledge about the build system 
I was just blindly moving library orders around without looking into the 
semantical meaning of these dependencies.

> With this patch, both static and shared builds work for me:

Confirmed. Works for me too.

The only thing that I am a bit concerned about is, why is the HTTP ETag 
functionality now part of the core, and not part of the http module 
anymore? Isn't this somewhat adverse to other efforts to move more code 
from core to modules?

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 12:11 PM, Micha Lenk <mi...@lenk.info> wrote:
>
> On 05/24/2018 12:00 PM, Yann Ylavic wrote:
>
>> I think "core" shouldn't depend on a module (even builtin), for
>> instance ap_set_{last_modified,accept_range,content_length,...} also
>> used by the core are defined in "server/protocol.c".
>>
>> WDYT?
>
> Well, first things first. Let's first fix trunk to be buildable again on
> build systems that really only link the needed symbols and thus rely on the
> correct library order during linking.

I think this is *the* dependency issue, the order in
PROGRAM_DEPENDENCIES should make modules depend on core and not the
other way around.

With this patch, both static and shared builds work for me:

Index: Makefile.in
===================================================================
--- Makefile.in    (revision 1832123)
+++ Makefile.in    (working copy)
@@ -7,9 +7,9 @@ PROGRAM_SOURCES      = modules.c
 PROGRAM_LDADD        = buildmark.o $(HTTPD_LDFLAGS)
$(PROGRAM_DEPENDENCIES) $(HTTPD_LIBS) $(EXTRA_LIBS) $(AP_LIBS) $(LIBS)
 PROGRAM_PRELINK      = $(COMPILE) -c $(top_srcdir)/server/buildmark.c
 PROGRAM_DEPENDENCIES = \
+  $(MPM_LIB) \
+  $(BUILTIN_LIBS) \
   server/libmain.la \
-  $(BUILTIN_LIBS) \
-  $(MPM_LIB) \
   os/$(OS_DIR)/libos.la

 sbin_PROGRAMS   = $(PROGRAM_NAME)
Index: modules/http/http_etag.c
===================================================================
--- modules/http/http_etag.c    (revision 1832123)
+++ modules/http/http_etag.c    (working copy)
@@ -14,6 +14,7 @@
  * limitations under the License.
  */

+#ifdef INCLUDE_FROM_CORE
 #include "apr_strings.h"
 #include "apr_thread_proc.h"    /* for RLIMIT stuff */

@@ -218,3 +219,4 @@ AP_DECLARE(void) ap_set_etag(request_rec *r)

     apr_table_setn(r->headers_out, "ETag", etag);
 }
+#endif
Index: server/protocol.c
===================================================================
--- server/protocol.c    (revision 1832123)
+++ server/protocol.c    (working copy)
@@ -164,6 +164,9 @@ AP_DECLARE(void) ap_set_content_length(request_rec
                    apr_off_t_toa(r->pool, clength));
 }

+#define INCLUDE_FROM_CORE
+#include "../modules/http/http_etag.c"
+
 /*
  * Return the latest rational time from a request/mtime (modification time)
  * pair.  We return the mtime unless it's in the future, in which case we
_

Of course it should be a real move of the code...

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Yann,

FWIW I've found a very good explanation of what's going on during 
linking and why the library order in static linking is so important.
https://eli.thegreenplace.net/2013/07/09/library-order-in-static-linking

On 05/24/2018 12:00 PM, Yann Ylavic wrote:
> Looks like the right order to me, however it fails with shared modules
> because "server/core.c" (in libmain) uses ap_set_etag() function from
> "modules/http/http_etag.c" (in BUILTIN_LIBS's libmod_http).

This could indicate that the default_handler() function which calls 
ap_set_etag() should better be moved to some file in modules/http/. Of 
course that opens a totally different can of worms...

> I think "core" shouldn't depend on a module (even builtin), for
> instance ap_set_{last_modified,accept_range,content_length,...} also
> used by the core are defined in "server/protocol.c".
> 
> WDYT?

Well, first things first. Let's first fix trunk to be buildable again on 
build systems that really only link the needed symbols and thus rely on 
the correct library order during linking.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 11:28 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, May 24, 2018 at 11:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Thu, May 24, 2018 at 11:06 AM, Micha Lenk <mi...@lenk.info> wrote:
>>>
>>> On 05/24/2018 10:41 AM, Yann Ylavic wrote:
>>>
>>>> Btw, as Jacob noted, the attached patch seems to work for me (even
>>>> without the above option).
>>>
>>> Yes, for me too, except that the linker problem with undefined symbols now
>>> seems to shift to the modules. I had to disable a few modules
>>> (--enable-mods-static=most --disable-apreq --disable-proxy-fcgi
>>> --disable-session-cookie --disable-session-dbd --disable-dav) to finally get
>>> to a successful build.
>>
>> Oh indeed, the correct order seems to be:
>>
>> PROGRAM_DEPENDENCIES = \
>>   $(MPM_LIB) \
>>   $(BUILTIN_LIBS) \
>>   server/libmain.la \
>>   os/$(OS_DIR)/libos.la
>
> Argh no, it now fails with --enable-mods-shared=...

Looks like the right order to me, however it fails with shared modules
because "server/core.c" (in libmain) uses ap_set_etag() function from
"modules/http/http_etag.c" (in BUILTIN_LIBS's libmod_http).

I think "core" shouldn't depend on a module (even builtin), for
instance ap_set_{last_modified,accept_range,content_length,...} also
used by the core are defined in "server/protocol.c".

WDYT?

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 11:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, May 24, 2018 at 11:06 AM, Micha Lenk <mi...@lenk.info> wrote:
>>
>> On 05/24/2018 10:41 AM, Yann Ylavic wrote:
>>
>>> Btw, as Jacob noted, the attached patch seems to work for me (even
>>> without the above option).
>>
>> Yes, for me too, except that the linker problem with undefined symbols now
>> seems to shift to the modules. I had to disable a few modules
>> (--enable-mods-static=most --disable-apreq --disable-proxy-fcgi
>> --disable-session-cookie --disable-session-dbd --disable-dav) to finally get
>> to a successful build.
>
> Oh indeed, the correct order seems to be:
>
> PROGRAM_DEPENDENCIES = \
>   $(MPM_LIB) \
>   $(BUILTIN_LIBS) \
>   server/libmain.la \
>   os/$(OS_DIR)/libos.la

Argh no, it now fails with --enable-mods-shared=...

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Yann,

On 05/24/2018 11:23 AM, Yann Ylavic wrote:
>> Yes, for me too, except that the linker problem with undefined symbols now
>> seems to shift to the modules. I had to disable a few modules
>> (--enable-mods-static=most --disable-apreq --disable-proxy-fcgi
>> --disable-session-cookie --disable-session-dbd --disable-dav) to finally get
>> to a successful build.
>
> Oh indeed, the correct order seems to be:

I'd rather expect some fixes in the affected modules linker order.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, May 24, 2018 at 11:06 AM, Micha Lenk <mi...@lenk.info> wrote:
>
> On 05/24/2018 10:41 AM, Yann Ylavic wrote:
>
>> Btw, as Jacob noted, the attached patch seems to work for me (even
>> without the above option).
>
> Yes, for me too, except that the linker problem with undefined symbols now
> seems to shift to the modules. I had to disable a few modules
> (--enable-mods-static=most --disable-apreq --disable-proxy-fcgi
> --disable-session-cookie --disable-session-dbd --disable-dav) to finally get
> to a successful build.

Oh indeed, the correct order seems to be:

PROGRAM_DEPENDENCIES = \
  $(MPM_LIB) \
  $(BUILTIN_LIBS) \
  server/libmain.la \
  os/$(OS_DIR)/libos.la


Regards,
Yann.

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Hi Yann,

On 05/24/2018 10:41 AM, Yann Ylavic wrote:
>> ./configure --prefix=/home/mlenk/Upstream/Apache/target
>> --with-apr=/home/mlenk/Upstream/Apache/target
>> --with-apr-util=/home/mlenk/Upstream/Apache/target --with-mpm=worker
>> --with-mpms-shared=all --enable-mods-static=most --enable-load-all-modules
>
> This should be --enable-mpms-shared=all (not --with-...).

Drat. What a good catch! I should have enjoyed a coffee before trying 
the suggested work around.

> Btw, as Jacob noted, the attached patch seems to work for me (even
> without the above option).

Yes, for me too, except that the linker problem with undefined symbols 
now seems to shift to the modules. I had to disable a few modules 
(--enable-mods-static=most --disable-apreq --disable-proxy-fcgi 
--disable-session-cookie --disable-session-dbd --disable-dav) to finally 
get to a successful build.

Thanks for your help.

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Yann Ylavic <yl...@gmail.com>.
Hi Micha,

On Thu, May 24, 2018 at 10:29 AM, Micha Lenk <mi...@lenk.info> wrote:
>
> I tried several combinations of --with-mpm=worker and
> --with-mpms-shared=all, but none of them worked (all attempts with "make
> clean"). The most recent attempt used the following ./configure command
> line:
>
> ./configure --prefix=/home/mlenk/Upstream/Apache/target
> --with-apr=/home/mlenk/Upstream/Apache/target
> --with-apr-util=/home/mlenk/Upstream/Apache/target --with-mpm=worker
> --with-mpms-shared=all --enable-mods-static=most --enable-load-all-modules

This should be --enable-mpms-shared=all (not --with-...).

Btw, as Jacob noted, the attached patch seems to work for me (even
without the above option).

Regards,
Yann.

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
On 05/23/2018 10:21 PM, Christophe Jaillet wrote:
> I can reproduce the issue if I don't pass any --enable-mpms-shared 
> paramater to ./configure.
> Having --with-mpm=xx only also triggers the building issue.
> 
> What is your ./configure command line?

The initial ./configure command line was:

./configure --prefix=/home/build/target --with-apr=/home/build/target 
--with-apr-util=/home/build/target --enable-modules=most

> Can you try to add --enable-mpms-shared=event (or =all) and re-configure 
> and build?

I tried several combinations of --with-mpm=worker and 
--with-mpms-shared=all, but none of them worked (all attempts with "make 
clean"). The most recent attempt used the following ./configure command 
line:

./configure --prefix=/home/mlenk/Upstream/Apache/target 
--with-apr=/home/mlenk/Upstream/Apache/target 
--with-apr-util=/home/mlenk/Upstream/Apache/target --with-mpm=worker 
--with-mpms-shared=all --enable-mods-static=most --enable-load-all-modules

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Christophe Jaillet <ch...@wanadoo.fr>.
Le 23/05/2018 à 20:52, Micha Lenk a écrit :
> Am 23.05.2018 um 20:18 schrieb Marion et Christophe JAILLET:
>> Could you please try to 'make clean' and 'make' to see if you still have
>> the build issue?
> 
> No change. :(
> 
> Regards,
> Micha
> 
I can reproduce the issue if I don't pass any --enable-mpms-shared 
paramater to ./configure.
Having --with-mpm=xx only also triggers the building issue.

What is your ./configure command line?
Can you try to add --enable-mpms-shared=event (or =all) and re-configure 
and build?

CJ


Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
Am 23.05.2018 um 20:18 schrieb Marion et Christophe JAILLET:
> Could you please try to 'make clean' and 'make' to see if you still have
> the build issue?

No change. :(

Regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Marion et Christophe JAILLET <ch...@wanadoo.fr>.

Le 23/05/2018 à 14:48, Micha Lenk a écrit :
> On 05/25/2017 11:44 PM, Jacob Champion wrote (almost a year ago):
>> Last week I had a personal hackathon since I couldn't make it out to 
>> ApacheCon. As a result there is now a C-language unit test suite 
>> available in branches/httpdunit (based on trunk). I've tested it with 
>> a Windows+CMake toolchain as well as an Ubuntu+autoconf toolchain.
>>
>> The suite itself is based on Check, which is a testing library I've 
>> had some success with in the past. It's supported on a wide variety 
>> of platforms and has a nice feature of running each test in a 
>> separate process space, so a crash doesn't derail the entire suite. 
>> (Note: Check is LGPL.) The build system has been augmented slightly 
>> to generate some of the more tedious boilerplate code.
>>
>> If you want to give it a try, just install Check (and, if using the 
>> configure scripts, make sure Check is visible via pkg-config). The 
>> test suite will then automatically be added to the default targets. 
>> Once everything builds you just run the suite directly with
>>
>>      $ ./test/httpdunit
>>
>> As a Check binary, it has multiple knobs to control which tests run 
>> and how the reporting is done, but by default it just runs all the 
>> tests and prints TAP to stdout.
>>
>> The example tests that are currently running are testing a new API 
>> for strict Base64 decoding. Right now it's a feature without a 
>> client; I included it here because it was a good showcase of the test 
>> suite (see test/unit/base64.c for the test case code).
>>
>> Let me know what you think!
>
> I totally like the idea of having unit tests for httpd, so thanks a 
> lot for adding them!
>
> I recently struggled compiling trunk locally on Debian 9.4 because of 
> these unit tests. Apparently it fails to link test/httpdunit because 
> of undefined references to ap_queue_info_push_pool and other symbols. 
> See the attached build log for more details.
>
> Does anybody have an idea what went wrong? Am I missing something?
> How does buildbot build?
>
> Best regards,
> Micha
Hi,
did you try to make a 'make clean'?
As explained in another message, 'test/httpdunit' is built if the 
'check' library is found on the system. However, the .o files don't seem 
to be updated when the code of httpd is updated. So the (previous) .o 
files could have references to some functions that do not exists any more.

Could you please try to 'make clean' and 'make' to see if you still have 
the build issue?

CJ

Re: Experimental C unit test suite available for hacking

Posted by Micha Lenk <mi...@lenk.info>.
On 05/25/2017 11:44 PM, Jacob Champion wrote (almost a year ago):
> Last week I had a personal hackathon since I couldn't make it out to 
> ApacheCon. As a result there is now a C-language unit test suite 
> available in branches/httpdunit (based on trunk). I've tested it with a 
> Windows+CMake toolchain as well as an Ubuntu+autoconf toolchain.
> 
> The suite itself is based on Check, which is a testing library I've had 
> some success with in the past. It's supported on a wide variety of 
> platforms and has a nice feature of running each test in a separate 
> process space, so a crash doesn't derail the entire suite. (Note: Check 
> is LGPL.) The build system has been augmented slightly to generate some 
> of the more tedious boilerplate code.
> 
> If you want to give it a try, just install Check (and, if using the 
> configure scripts, make sure Check is visible via pkg-config). The test 
> suite will then automatically be added to the default targets. Once 
> everything builds you just run the suite directly with
> 
>      $ ./test/httpdunit
> 
> As a Check binary, it has multiple knobs to control which tests run and 
> how the reporting is done, but by default it just runs all the tests and 
> prints TAP to stdout.
> 
> The example tests that are currently running are testing a new API for 
> strict Base64 decoding. Right now it's a feature without a client; I 
> included it here because it was a good showcase of the test suite (see 
> test/unit/base64.c for the test case code).
> 
> Let me know what you think!

I totally like the idea of having unit tests for httpd, so thanks a lot 
for adding them!

I recently struggled compiling trunk locally on Debian 9.4 because of 
these unit tests. Apparently it fails to link test/httpdunit because of 
undefined references to ap_queue_info_push_pool and other symbols. See 
the attached build log for more details.

Does anybody have an idea what went wrong? Am I missing something?
How does buildbot build?

Best regards,
Micha

Re: Experimental C unit test suite available for hacking

Posted by Graham Leggett <mi...@sharp.fm>.
On 25 May 2017, at 11:44 PM, Jacob Champion <ch...@gmail.com> wrote:

> Last week I had a personal hackathon since I couldn't make it out to ApacheCon. As a result there is now a C-language unit test suite available in branches/httpdunit (based on trunk). I've tested it with a Windows+CMake toolchain as well as an Ubuntu+autoconf toolchain.
> 
> The suite itself is based on Check, which is a testing library I've had some success with in the past. It's supported on a wide variety of platforms and has a nice feature of running each test in a separate process space, so a crash doesn't derail the entire suite. (Note: Check is LGPL.) The build system has been augmented slightly to generate some of the more tedious boilerplate code.
> 
> If you want to give it a try, just install Check (and, if using the configure scripts, make sure Check is visible via pkg-config). The test suite will then automatically be added to the default targets. Once everything builds you just run the suite directly with
> 
>    $ ./test/httpdunit
> 
> As a Check binary, it has multiple knobs to control which tests run and how the reporting is done, but by default it just runs all the tests and prints TAP to stdout.
> 
> The example tests that are currently running are testing a new API for strict Base64 decoding. Right now it's a feature without a client; I included it here because it was a good showcase of the test suite (see test/unit/base64.c for the test case code).
> 
> Let me know what you think!

Love the idea of a test suite written in the same language as the server itself.

It means that anyone who updates the server can use the same skills to update the tests.

Regards,
Graham
—