feat: shared site testing script and move to linkinator by mcdurdin · Pull Request #97 · keymanapp/shared-sites

mcdurdin · 2026-06-15T08:40:32Z

Relates-to: keymanapp/keyman.com#788
Test-bot: skip

keymanapp-test-bot · 2026-06-15T08:40:41Z

User Test Results

User tests are not required

Relates-to: keymanapp/keyman.com#788 Test-bot: skip

ermshiperete · 2026-06-15T16:41:55Z

+  local CONTAINER_PORT="$2"
+  local TEST_PATH="$3"
+  shift 3
+  local SKIP_PATHS=("$*")


Suggested change

local SKIP_PATHS=("$*")

local SKIP_PATHS=("$@")

ermshiperete · 2026-06-15T16:42:58Z

+  shift 2
+  local skipPaths skip skipParams=()
+  if [[ $# -gt 1 ]]; then
+    skipPaths=("$*")


Suggested change

skipPaths=("$*")

skipPaths=("$@")

ermshiperete · 2026-06-15T16:46:07Z

+  local LINK_RESULT
+  echo "TIER_TEST" > tier.txt
+
+  # from commands.inc.sh


Which commands.inc.sh ?

Good catch. Left behind from a refactor

ermshiperete · 2026-06-15T16:50:25Z

@@ -0,0 +1,93 @@
+# shellcheck shell=bash


Suggested change

# shellcheck shell=bash

# shellcheck shell=bash

# Keyman is copyright (C) SIL Global. MIT License.

ermshiperete · 2026-06-15T16:51:05Z

@@ -1,5 +1,7 @@
 # Common docker functions.


Suggested change

# Common docker functions.

# shellcheck shell=bash

# Keyman is copyright (C) SIL Global. MIT License.

#

# Common docker functions.

ermshiperete · 2026-06-15T16:52:07Z

+#
+function do_test_unit_tests() {
+  local CONTAINER="$1"
+  docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[*]}"


Suggested change

docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[*]}"

docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[@]}"

I don't think we can use ${x[@]} here because that will quote at the wrong level -- it needs to remain inside the quotes for the parameter following sh -c.

ermshiperete · 2026-06-15T17:01:43Z

+  shift 3
+  local SKIP_PATHS=("$*")
+
+  local LINK_RESULT


This needs to be initialized, otherwise we return uninitialized value if we don't do link checks.

Suggested change

local LINK_RESULT

local LINK_RESULT=0

ermshiperete · 2026-06-15T17:03:32Z

+  local testPath="$2"
+  shift 2
+  local skipPaths skip skipParams=()
+  if [[ $# -gt 1 ]]; then


Suggested change

if [[ $# -gt 1 ]]; then

if [[ $# -ge 1 ]]; then

Ah, bingo, if I have do_test_links a b "$*" rather than do_test_links a b "$@", then I end up with a spurious blank parameter...

$* vs $@ is such an annoying distinction 😁

ermshiperete · 2026-06-15T17:09:05Z

+#
+function do_test_print_container_error_logs() {
+  local CONTAINER="$1"
+  if docker container logs "${CONTAINER}" --since "${TEST_START_TIME}" 2>&1 | grep -q 'php7'; then


Can we add some check somewhere that we're still running the expected PHP version that outputs php7?

Makes me a bit nervous that PHP gets upgraded and we forget to upgrade this string which then causes silent failure (or rather non-failure) of this function.

I tested against PHP 8 and the version number has been removed from the string. Have gone with the following (tested) string which works with PHP 7 and 8 running through Apache:

'\[php7?:(error|warn|notice)\]'

The format of messages may of course change again at any time in the future but we can only do so much...

ermshiperete · 2026-06-15T17:11:07Z

+  local SKIP_PATHS=("$*")
+
+  local LINK_RESULT
+  echo "TIER_TEST" > tier.txt


What's the purpose of tier.txt ?

This is used to specify the tier across server and script contexts consistently (as env vars may not be available in http server context). See:

shared-sites/bootstrap.inc.sh

Lines 209 to 216 in 5ac4983

# Get the site BUILDER_TIER and use that to determine builder script environment

if [[ ! -z ${KEYMANHOSTS_TIER+x} ]]; then

BUILDER_TIER="${KEYMANHOSTS_TIER}"

elif [[ -f "$(dirname "$THIS_SCRIPT")/tier.txt" ]]; then

BUILDER_TIER=$(cat "$(dirname "$THIS_SCRIPT")/tier.txt")

else

BUILDER_TIER=TIER_DEVELOPMENT

fi

shared-sites/_common/KeymanHosts.php

Lines 128 to 136 in 2c8091d

if(isset($env['KEYMANHOSTS_TIER']) && in_array($env['KEYMANHOSTS_TIER'],

[KeymanHosts::TIER_DEVELOPMENT, KeymanHosts::TIER_STAGING,

KeymanHosts::TIER_PRODUCTION, KeymanHosts::TIER_TEST])) {

$this->tier = $env['KEYMANHOSTS_TIER'];

} else if(file_exists(__DIR__ . '/../tier.txt')) {

$this->tier = trim(file_get_contents(__DIR__ . '/../tier.txt'));

} else {

$this->tier = KeymanHosts::TIER_DEVELOPMENT;

}

Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>

mcdurdin

Thanks for the detailed review! I think I've addressed all the code changes.

mcdurdin · 2026-06-16T05:08:49Z

+  local SKIP_PATHS=("$*")
+
+  local LINK_RESULT
+  echo "TIER_TEST" > tier.txt


This is used to specify the tier across server and script contexts consistently (as env vars may not be available in http server context). See:

shared-sites/bootstrap.inc.sh

Lines 209 to 216 in 5ac4983

# Get the site BUILDER_TIER and use that to determine builder script environment

if [[ ! -z ${KEYMANHOSTS_TIER+x} ]]; then

BUILDER_TIER="${KEYMANHOSTS_TIER}"

elif [[ -f "$(dirname "$THIS_SCRIPT")/tier.txt" ]]; then

BUILDER_TIER=$(cat "$(dirname "$THIS_SCRIPT")/tier.txt")

else

BUILDER_TIER=TIER_DEVELOPMENT

fi

shared-sites/_common/KeymanHosts.php

Lines 128 to 136 in 2c8091d

if(isset($env['KEYMANHOSTS_TIER']) && in_array($env['KEYMANHOSTS_TIER'],

[KeymanHosts::TIER_DEVELOPMENT, KeymanHosts::TIER_STAGING,

KeymanHosts::TIER_PRODUCTION, KeymanHosts::TIER_TEST])) {

$this->tier = $env['KEYMANHOSTS_TIER'];

} else if(file_exists(__DIR__ . '/../tier.txt')) {

$this->tier = trim(file_get_contents(__DIR__ . '/../tier.txt'));

} else {

$this->tier = KeymanHosts::TIER_DEVELOPMENT;

}

mcdurdin · 2026-06-16T05:09:31Z

+  local LINK_RESULT
+  echo "TIER_TEST" > tier.txt
+
+  # from commands.inc.sh


Good catch. Left behind from a refactor

mcdurdin · 2026-06-16T05:13:23Z

+#
+function do_test_unit_tests() {
+  local CONTAINER="$1"
+  docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[*]}"


I don't think we can use ${x[@]} here because that will quote at the wrong level -- it needs to remain inside the quotes for the parameter following sh -c.

mcdurdin · 2026-06-16T05:17:08Z

+  local testPath="$2"
+  shift 2
+  local skipPaths skip skipParams=()
+  if [[ $# -gt 1 ]]; then


Ah, bingo, if I have do_test_links a b "$*" rather than do_test_links a b "$@", then I end up with a spurious blank parameter...

$* vs $@ is such an annoying distinction 😁

mcdurdin · 2026-06-16T05:50:24Z

+#
+function do_test_print_container_error_logs() {
+  local CONTAINER="$1"
+  if docker container logs "${CONTAINER}" --since "${TEST_START_TIME}" 2>&1 | grep -q 'php7'; then


I tested against PHP 8 and the version number has been removed from the string. Have gone with the following (tested) string which works with PHP 7 and 8 running through Apache:

'\[php7?:(error|warn|notice)\]'

The format of messages may of course change again at any time in the future but we can only do so much...

mcdurdin requested a review from ermshiperete June 15, 2026 08:40

keyman-status added this to Keyman Jun 15, 2026

github-actions Bot added the feat label Jun 15, 2026

keymanapp-test-bot Bot added this to the A19S31 milestone Jun 15, 2026

github-project-automation Bot moved this to Todo in Keyman Jun 15, 2026

mcdurdin force-pushed the feat/linkinator-and-central-test-script branch from 741832a to c855186 Compare June 15, 2026 08:54

feat: shared site testing script and move to linkinator

df4837e

Relates-to: keymanapp/keyman.com#788 Test-bot: skip

mcdurdin force-pushed the feat/linkinator-and-central-test-script branch from c855186 to df4837e Compare June 15, 2026 08:59

ermshiperete requested changes Jun 15, 2026

View reviewed changes

chore: address review comments

25765bf

Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>

mcdurdin commented Jun 16, 2026

View reviewed changes

ermshiperete approved these changes Jun 16, 2026

View reviewed changes

mcdurdin merged commit 1998f79 into main Jun 16, 2026
3 checks passed

mcdurdin deleted the feat/linkinator-and-central-test-script branch June 16, 2026 13:26

github-project-automation Bot moved this from Todo to Done in Keyman Jun 16, 2026

	# shellcheck shell=bash
	# shellcheck shell=bash
	# Keyman is copyright (C) SIL Global. MIT License.

	docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[*]}"
	docker exec "${CONTAINER}" sh -c "vendor/bin/phpunit --testdox ${builder_extra_params[@]}"

	# Get the site BUILDER_TIER and use that to determine builder script environment
	if [[ ! -z ${KEYMANHOSTS_TIER+x} ]]; then
	BUILDER_TIER="${KEYMANHOSTS_TIER}"
	elif [[ -f "$(dirname "$THIS_SCRIPT")/tier.txt" ]]; then
	BUILDER_TIER=$(cat "$(dirname "$THIS_SCRIPT")/tier.txt")
	else
	BUILDER_TIER=TIER_DEVELOPMENT
	fi

	if(isset($env['KEYMANHOSTS_TIER']) && in_array($env['KEYMANHOSTS_TIER'],
	[KeymanHosts::TIER_DEVELOPMENT, KeymanHosts::TIER_STAGING,
	KeymanHosts::TIER_PRODUCTION, KeymanHosts::TIER_TEST])) {
	$this->tier = $env['KEYMANHOSTS_TIER'];
	} else if(file_exists(__DIR__ . '/../tier.txt')) {
	$this->tier = trim(file_get_contents(__DIR__ . '/../tier.txt'));
	} else {
	$this->tier = KeymanHosts::TIER_DEVELOPMENT;
	}

Uh oh!

Uh oh!

Conversation

mcdurdin commented Jun 15, 2026

Uh oh!

keymanapp-test-bot Bot commented Jun 15, 2026

User Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mcdurdin Jun 16, 2026 •

edited

Loading

mcdurdin Jun 16, 2026 •

edited

Loading