From 71c30cb603192015a94676424458bf3b0e9e654d Mon Sep 17 00:00:00 2001
From: Stefan Eissing <stefan@eissing.org>
Date: Mon, 3 Apr 2023 11:58:21 +0200
Subject: [PATCH] Improved httpd detection for pytest

- better error messages when not found/complete
- handling of `--without-test-httpd`
---
 configure.ac              | 26 +++++++++++++++++++-------
 tests/http/conftest.py    |  6 ++++--
 tests/http/testenv/env.py | 20 ++++++++++++++------
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/configure.ac b/configure.ac
index e351d1e184d1ca..6f3575023e2b51 100644
--- a/configure.ac
+++ b/configure.ac
@@ -323,10 +323,11 @@ AC_SUBST(CADDY)
 
 dnl we'd like a httpd+apachectl as test server
 dnl
+HTTPD_ENABLED="maybe"
 AC_ARG_WITH(test-httpd, [AS_HELP_STRING([--with-test-httpd=PATH],
                          [where to find httpd/apache2 for testing])],
   [request_httpd=$withval], [request_httpd=check])
-if test x"$request_httpd" = "xcheck"; then
+if test x"$request_httpd" = "xcheck" -o x"$request_httpd" = "xyes"; then
   if test -x "/usr/sbin/apache2" -a -x "/usr/sbin/apache2ctl"; then
     # common location on distros (debian/ubuntu)
     HTTPD="/usr/sbin/apache2"
@@ -334,6 +335,7 @@ if test x"$request_httpd" = "xcheck"; then
     AC_PATH_PROG([APXS], [apxs])
     if test "x$APXS" != "x"; then
       AC_MSG_NOTICE([apache2-dev not installed, httpd tests disabled])
+      HTTPD_ENABLED="no"
     fi
   else
     AC_PATH_PROG([HTTPD], [httpd])
@@ -343,26 +345,36 @@ if test x"$request_httpd" = "xcheck"; then
     AC_PATH_PROG([APACHECTL], [apachectl])
     AC_PATH_PROG([APXS], [apxs])
     if test "x$HTTPD" = "x" -o "x$APACHECTL" = "x"; then
-      AC_MSG_NOTICE([httpd/apache2 not in PATH, httpd tests disabled])
+      AC_MSG_NOTICE([httpd/apache2 not in PATH, http tests disabled])
+      HTTPD_ENABLED="no"
     fi
     if test "x$APXS" = "x"; then
-      AC_MSG_NOTICE([apxs not in PATH, httpd tests disabled])
+      AC_MSG_NOTICE([apxs not in PATH, http tests disabled])
+      HTTPD_ENABLED="no"
     fi
   fi
-else
+elif test x"$request_httpd" != "xno"; then
   HTTPD="${request_httpd}/bin/httpd"
   APACHECTL="${request_httpd}/bin/apachectl"
   APXS="${request_httpd}/bin/apxs"
   if test ! -x "${HTTPD}"; then
-    AC_MSG_NOTICE([httpd not found as ${HTTPD}, httpd tests disabled])
+    AC_MSG_NOTICE([httpd not found as ${HTTPD}, http tests disabled])
+    HTTPD_ENABLED="no"
   elif test ! -x "${APACHECTL}"; then
-    AC_MSG_NOTICE([apachectl not found as ${APACHECTL}, httpd tests disabled])
+    AC_MSG_NOTICE([apachectl not found as ${APACHECTL}, http tests disabled])
+    HTTPD_ENABLED="no"
   elif test ! -x "${APXS}"; then
-    AC_MSG_NOTICE([apxs not found as ${APXS}, httpd tests disabled])
+    AC_MSG_NOTICE([apxs not found as ${APXS}, http tests disabled])
+    HTTPD_ENABLED="no"
   else
     AC_MSG_NOTICE([using HTTPD=$HTTPD for tests])
   fi
 fi
+if test x"$HTTPD_ENABLED" = "xno"; then
+  HTTPD=""
+  APACHECTL=""
+  APXS=""
+fi
 AC_SUBST(HTTPD)
 AC_SUBST(APACHECTL)
 AC_SUBST(APXS)
diff --git a/tests/http/conftest.py b/tests/http/conftest.py
index eecc0c089239ba..22386b94d7d85a 100644
--- a/tests/http/conftest.py
+++ b/tests/http/conftest.py
@@ -57,9 +57,11 @@ def log_global_env_facts(record_testsuite_property, env):
 @pytest.fixture(scope='package')
 def httpd(env) -> Httpd:
     httpd = Httpd(env=env)
-    assert httpd.exists(), f'httpd not found: {env.httpd}'
+    if not httpd.exists():
+        pytest.skip(f'httpd not found: {env.httpd}')
     httpd.clear_logs()
-    assert httpd.start()
+    if not httpd.start():
+        pytest.fail(f'failed to start httpd: {env.httpd}')
     yield httpd
     httpd.stop()
 
diff --git a/tests/http/testenv/env.py b/tests/http/testenv/env.py
index 4c42931af584c5..f09b7174951374 100644
--- a/tests/http/testenv/env.py
+++ b/tests/http/testenv/env.py
@@ -166,11 +166,15 @@ def __init__(self):
     @property
     def httpd_version(self):
         if self._httpd_version is None and self.apxs is not None:
-            p = subprocess.run(args=[self.apxs, '-q', 'HTTPD_VERSION'],
-                               capture_output=True, text=True)
-            if p.returncode != 0:
-                raise Exception(f'{self.apxs} failed to query HTTPD_VERSION: {p}')
-            self._httpd_version = p.stdout.strip()
+            try:
+                p = subprocess.run(args=[self.apxs, '-q', 'HTTPD_VERSION'],
+                                   capture_output=True, text=True)
+                if p.returncode != 0:
+                    log.error(f'{self.apxs} failed to query HTTPD_VERSION: {p}')
+                else:
+                    self._httpd_version = p.stdout.strip()
+            except Exception as e:
+                log.error(f'{self.apxs} failed to run: {e}')
         return self._httpd_version
 
     def _versiontuple(self, v):
@@ -178,6 +182,8 @@ def _versiontuple(self, v):
         return tuple(map(int, v.split('.')))
 
     def httpd_is_at_least(self, minv):
+        if self.httpd_version is None:
+            return False
         hv = self._versiontuple(self.httpd_version)
         return hv >= self._versiontuple(minv)
 
@@ -188,12 +194,14 @@ def is_complete(self) -> bool:
                os.path.isfile(self.apxs)
 
     def get_incomplete_reason(self) -> Optional[str]:
+        if self.httpd is None or len(self.httpd.strip()) == 0:
+            return f'httpd not configured, see `--with-test-httpd=<path>`'
         if not os.path.isfile(self.httpd):
             return f'httpd ({self.httpd}) not found'
         if not os.path.isfile(self.apachectl):
             return f'apachectl ({self.apachectl}) not found'
         if self.apxs is None:
-            return f"apxs (provided by apache2-dev) not found"
+            return f"command apxs not found (commonly provided in apache2-dev)"
         if not os.path.isfile(self.apxs):
             return f"apxs ({self.apxs}) not found"
         return None