Re: Disabling src/test/[ssl|ldap] when not building with SSL/LDAP support

On Sat, Feb 10, 2018 at 1:07 PM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Feb 08, 2018 at 09:28:10AM -0500, Tom Lane wrote:
>> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>> That seems like possibly not such a great idea. If somebody were using
>> a run of src/test/ssl to check their build, they would now get no
>> notification if they'd forgotten to build --with-openssl.
>>
>> Perhaps we could introduce some new targets, "check-if-enabled" or so,
>> that act like you want.
>
> Per this argument, we need to do something even for check and
> installcheck anyway, no? Except that what you are suggesting is to make
> the tests fail instead of silently being bypassed. Copying an
> expression you used recently, this boils down to not spend CPU cycles
> for nothing. The TAP tests showing in red all over your screen don't
> show any useful information either as one may be building with SSL
> support, and still getting failures because he/she is working on an
> SSL-related feature.
>
> I prefer making the tests personally not fail, as when building without
> SSL one needs to move down to run ./configure again, so he likely knows
> what is is doing. Bypassing them also has the advantage to not do
> failure check dances, particularly in bash when using temporarily set
> +e/-e to avoid a problem, so this makes things easier for most cases.

One complication with the LDAP tests is that
src/test/ldap/t/001_auth.pl has greater requirements than --with-ldap.
For example, on a Debian system you probably only need the
libldap2-dev package installed for --with-ldap to build, but
001_auth.pl also requires the slapd package to be installed (the
OpenLDAP server). On some other systems including macOS and maybe
AIX, a conforming LDAP client library (maybe OpenLDAP or maybe some
other implementation) is part of the base system but I don't think
slapd is.

I agree that it would be nice if the build farm (and my unofficial
patch tester for that matter) could automatically test the LDAP stuff
when running on a suitable system, but I think it would need to be
based not just on ifeq ($(with_ldap),yes) as you showed. I think the
test would need a way to report that it can't find slapd so it can't
run, but not consider that to be a hard failure. Or something like
that...

It might also be a good idea if you could tell 001_auth.pl where slapd
and openssl (used by 001_auth.pl to make certificates) are through
environment variables, in case anyone wants to run those tests against
an installation other than one in the hardcoded paths we told it about
for Linux, macOS (with Brew) and FreeBSD.

--
Thomas Munro
http://www.enterprisedb.com