Ludovic Courtès <ludo@gnu.org> writes:

>> +  #:export (<apache-httpd-configuration>
>> +            apache-httpd-configuration
>> +            apache-httpd-configuration?
>> +            apache-httpd-configuration-package
>> +            apache-httpd-configuration-pid-file
>> +            apache-httpd-configuration-config
>
> In this context I think ‘httpd-’ would be good enough as a prefix.

Ok.

>> +(define %default-apache-httpd-modules
>> +  (map (match-lambda
>> +         ((name file)
>> +          (apache-httpd-load-module
>> +           (name name)
>> +           (file file))))
>> +       '(("authn_file_module" "modules/mod_authn_file.so")
>> +         ("authn_core_module" "modules/mod_authn_core.so")
>> +         ("authz_host_module" "modules/mod_authz_host.so")
>
> I think having this list here is the right approach.  However could you
> write where it comes from?  I guess it’s equal to the current default in
> httpd, right?

Yeah, I think I copied the configuration from the httpd.conf file in the
httpd package.

> How often do you expect this to be updated?

I'm not sure, but probably infrequently.

>> +(define-record-type* <apache-httpd-config-file>
>> +  apache-httpd-config-file make-apache-httpd-config-file
>> +  apache-httpd-config-file?
>> +  (load-modules   apache-httpd-config-file-load-modules
>> +                  (default %default-apache-httpd-modules))
>
> Or “loaded-modules” or just “modules”?

Yeah, I'll look at changing this to modules.

>> +(define* (run-apache-httpd-test #:optional (http-port 8080))
>> +  "Run tests in %APACHE-HTTPD-OS, which has apache-httpd running and listening on
>> +HTTP-PORT."
>
> I wonder if we could abstract ‘run-nginx-test’ just enough so it can be
> used for both.  It’d need to take a service name, log file, and PID file
> as arguments.

That sounds good, I'll take a look.

> Otherwise LGTM, thanks!

Thanks for reviewing :)