maint: rework PHP error reporting for link tester

.bootstrap-registry

@@ -21,3 +21,4 @@ KeymanSentry.php
 KeymanVersion.php
 MarkdownHost.php
 tests.inc.sh
+tests.container.sh

.gitattributes

@@ -0,0 +1,10 @@
+# Always use LF for Dockerfile - otherwise HEREDOCs in Dockerfile
+# will not work on Windows - we end up with a filename `foo\r` for
+# the command
+# ```Dockerfile
+# RUN <<EOF cat > foo
+# echo "Hello world"
+# EOF
+# ```
+Dockerfile    text eol=lf
+*.sh          text eol=lf

_common/KeymanSentry.php

@@ -7,29 +7,16 @@
   require_once __DIR__ . '/KeymanHosts.php';
   require_once __DIR__ . '/Assets.php';
 
-  if(KeymanHosts::Instance()->Tier() == KeymanHosts::TIER_DEVELOPMENT ||
-     KeymanHosts::Instance()->Tier() == KeymanHosts::TIER_TEST) {
-    // For testing broken pages, we want to send HTTP 500 so that
-    // broken-link-checker will report it; in order to do this we
-    // need to cache the page output so that headers are not sent
-    // too early
-    ob_start();
-  }
-
   class KeymanSentry {
     static function init($dsn) {
       \Sentry\init([
         'dsn' => $dsn,
         'environment' => KeymanHosts::Instance()->TierName(),
+        // 'logger' => new \Sentry\Logger\DebugStdOutLogger(),
         'before_send' => function (\Sentry\Event $event) {
           // Don't send events from localhost or dev environments
           if (KeymanHosts::Instance()->Tier() == KeymanHosts::TIER_DEVELOPMENT ||
               KeymanHosts::Instance()->Tier() == KeymanHosts::TIER_TEST) {
-             if(headers_sent()) {
-              echo "<p>Fatal error: not setting 500 because headers already sent.</p>";
-             } else {
-              header("HTTP/1.1 500 Internal Server Error");
-             }
             return null;
           }
           return $event;

_common/docker.inc.sh

@@ -81,6 +81,9 @@ function build_docker_container() {
   builder_echo "Building using $BUILDER_CONFIGURATION configuration"
 
   # Download docker image. --mount option requires BuildKit
+  #
+  # note: --no-cache may be added here if trying to replicate broken ci locally
+  #       where Docker resources have been cached
   DOCKER_BUILDKIT=1 $CONTAINER_ENGINE build -t $IMAGE_NAME --build-arg BUILDER_CONFIGURATION="${BUILDER_CONFIGURATION}" $FILE $TARGET
 }
 
@@ -188,6 +191,7 @@ function test_docker_container() {
 
   local LINK_RESULT=0
   echo "TIER_TEST" > tier.txt
+  trap _docker_cleanup_test_docker_container ERR EXIT SIGINT SIGTERM
 
   # Similar pattern in ci.yml on sites
 
@@ -206,17 +210,70 @@ function test_docker_container() {
   if ! builder_has_option --no-link-check; then
     builder_echo blue "---- Testing links"
 
+    do_test_links_setup "${CONTAINER_DESC}"
+
     do_test_links "http://localhost:${CONTAINER_PORT}" "$TEST_PATH" "${SKIP_PATHS[@]}" || LINK_RESULT=$?
     builder_echo blue "Done checking links; linkinator exit code: ${LINK_RESULT}"
     do_test_print_link_report
 
     do_test_print_container_error_logs "${CONTAINER_DESC}"
+
+    do_test_links_cleanup "${CONTAINER_DESC}"
   fi
 
   rm tier.txt
   return "$LINK_RESULT"
 }
 
+_docker_cleanup_test_docker_container() {
+  rm -f "$THIS_SCRIPT_PATH/tier.txt"
+}
+
+#
+# Setup a temporary container for interactive composer updates
+#
+function docker_build_and_start_composer_container() {
+  local COMPOSER_ID=composer-temp
+  docker build -f Dockerfile --target composer-builder --tag $COMPOSER_ID .
+
+  local DOCKER_BINDING
+  if [[ $OSTYPE =~ msys|cygwin ]]; then
+    # Windows needs leading slashes for path
+    DOCKER_BINDING="//$(pwd):/var/www/html/"
+  else
+    DOCKER_BINDING="$(pwd):/var/www/html/"
+  fi
+
+  docker run -v "$DOCKER_BINDING" --name $COMPOSER_ID --user root --rm -d $COMPOSER_ID
+
+  echo
+  echo "Some of the following commands may be helpful:"
+  echo "  docker exec $COMPOSER_ID    composer audit"
+  echo "  docker exec $COMPOSER_ID    composer update"
+  echo "  docker exec $COMPOSER_ID    composer update --lock"
+  echo "  docker exec $COMPOSER_ID    composer require package-name"
+  echo "  docker exec $COMPOSER_ID    composer require package-name --dev"
+  echo
+}
+
+#
+# Cleanup the temporary container used for interactive composer updates
+# (including copying composer files)
+#
+function docker_stop_and_cleanup_composer_container() {
+  local COMPOSER_ID=composer-temp
+  # copy modified files to mounted volume:
+  docker exec $COMPOSER_ID    cp composer.lock //var/www/html/
+  docker exec $COMPOSER_ID    cp composer.json //var/www/html/
+  # cleanup
+  docker stop $COMPOSER_ID
+  docker rmi $COMPOSER_ID
+
+  builder_echo "Copied any changes from composer.json, composer.lock from init container into repo"
+  builder_echo "Note: 'build.sh build' will be required if changes have been made to /vendor by composer"
+}
+
+
 
 # Returns 0 if the specified container engine is available, 1 otherwise
 _is_container_engine() {

_common/tests.container.sh

@@ -0,0 +1,41 @@
+#!/bin/bash
+#
+# Keyman is copyright (C) SIL Global. MIT License.
+#
+# Scripts to run in container for link tests -- capture all PHP messages emitted
+# when each page is visited by the link checker, and report on errors
+#
+
+set -eu
+
+if [[ ${BUILDER_PLATFORM-x} != docker ]]; then
+  echo tests.container.sh should run only in the Docker container context
+  exit 1
+fi
+
+if [ $# -lt 1 ]; then
+  echo "usage: $0 setup|report|cleanup"
+  exit 65
+fi
+
+ERROR_LOG=/tmp/php_errors.log
+
+if [ "$1" == "setup" ]; then
+  cp /usr/local/etc/php/php.ini-development /usr/local/etc/php/php.ini
+  echo "error_log = $ERROR_LOG" >> /usr/local/etc/php/php.ini
+  # note: apache restart must run from docker host
+  rm -f $ERROR_LOG
+elif [ "$1" == "report" ]; then
+  if [ -f $ERROR_LOG ]; then
+    cat $ERROR_LOG
+    exit 1
+  fi
+elif [ "$1" == "cleanup" ]; then
+  cp /usr/local/etc/php/php.ini-development /usr/local/etc/php/php.ini
+  # note: apache restart must run from docker host
+else
+  echo "Invalid parameter"
+  exit 65
+fi
+
+exit 0

_common/tests.inc.sh

@@ -33,7 +33,7 @@ function do_test_lint() {
 ## Check links on live local server using linkinator
 #
 # Parameters
-# 1:       baseURL     the top level URL for the site
+# 1:       baseURL     the top level URL for the site, e.g. http://localhost:8053 (no terminating slash)
 # 2:       testPath    path under baseURL to start testing, e.g. /
 # 3[,4..]: skipPaths   list of paths (under baseURL) to skip crawling, optional
 #
@@ -48,6 +48,8 @@ function do_test_links() {
     skipParams+=(--skip "^${baseURL}${skip}")
   done
 
+  builder_echo "Testing links; --skip ^(?!${baseURL}) ${skipParams[*]}"
+
   npx https://github.com/keymanapp/linkinator \
     "${baseURL}${testPath}" \
     --clean-urls \
@@ -76,19 +78,25 @@ function do_test_print_link_report() {
 }
 
 # Scan logs recorded on container since start of tests to find any reported PHP
-# errors (note, depends on '[php#:xxxx]' marker string, where # = 7 for PHP7, omitted for PHP8)
+# errors
 #
 # Parameters
 # 1: CONTAINER     container_desc to run on
 #
 function do_test_print_container_error_logs() {
   local CONTAINER="$1"
-  if docker container logs "${CONTAINER}" --since "${TEST_START_TIME}" 2>&1 | grep -qP '\[php7?:(error|warn|notice)\]'; then
-    echo 'PHP reported errors or warnings:'
-    docker container logs "${CONTAINER}" --since "${TEST_START_TIME}" 2>&1 | grep -P '\[php7?:(error|warn|notice)\]'
-    return 1
-  else
-    echo 'No PHP errors found'
-    return 0
-  fi
+  docker exec "${CONTAINER}" //var/www/html/_common/tests.container.sh report
 }
+
+
+function do_test_links_setup() {
+  local CONTAINER="$1"
+  docker exec "${CONTAINER}" //var/www/html/_common/tests.container.sh setup
+  docker kill "${CONTAINER}" --signal="USR1"
+}
+
+function do_test_links_cleanup() {
+  local CONTAINER="$1"
+  docker exec "${CONTAINER}" //var/www/html/_common/tests.container.sh cleanup
+  docker kill "${CONTAINER}" --signal="USR1"
+}

bootstrap.inc.sh

@@ -215,7 +215,7 @@ else
   BUILDER_TIER=TIER_DEVELOPMENT
 fi
 
-if [[ "$BUILDER_TIER" == TIER_DEVELOPMENT ]]; then
+if [[ "$BUILDER_TIER" == TIER_DEVELOPMENT || "$BUILDER_TIER" == TIER_TEST ]]; then
   export KEYMAN_VERSION_ENVIRONMENT=local
 fi