tea icon indicating copy to clipboard operation
tea copied to clipboard
verified publisher icon FriendsOfTYPO3

Align our `runTests.sh` with the Core instance more

Open oliverklee opened this issue 2 months ago • 7 comments

Goal

As a developer, I want our runTests.sh and the Core runTests.sh to be as similar as possible so that porting changes from Core to Tea are less work.

Acceptance Criteria

  • [ ] We have Core changes in Gerrit (and Forge tickets) for all runTests.sh cleanup that we needed to fix Shellcheck warnings (as far as possible) and that make sense for the Core.
  • [ ] These changes are fine-grained so that they can be reviewed easily and accepted on an individual level.
  • [ ] We have a list of to-dos to undo some of our changes in our cleanup that do not make sense for the Core, allowing for easier copy'n'pasting. (These need to include removing some Shellcheck rules.)

oliverklee avatar Nov 03 '25 11:11 oliverklee

Documentation

Run shellcheck for core runTest.sh: docker run -it --init --rm --pull always --user 1000 -v ./:/project:ro -e 'SHELLCHECK_OPTS=-e SC2086' docker.io/koalaman/shellcheck:v0.10.0 /project/Bui ld/Scripts/runTests.sh

Result:

In /project/Build/Scripts/runTests.sh line 25:
   if [[ $? -gt 0 ]]; then
          ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.


In /project/Build/Scripts/runTests.sh line 190:
       if [[ $? -gt 0 ]]; then
              ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.


In /project/Build/Scripts/runTests.sh line 199:
       if [[ $? -gt 0 ]]; then
              ^-- SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.


In /project/Build/Scripts/runTests.sh line 499:
SUFFIX=$(echo $RANDOM)
       ^-------------^ SC2116 (style): Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'.


In /project/Build/Scripts/runTests.sh line 607:
if [ $(uname) != "Darwin" ] && [ ${CONTAINER_BIN} = "docker" ]; then
     ^------^ SC2046 (warning): Quote this to prevent word splitting.


In /project/Build/Scripts/runTests.sh line 675:
           COMMAND=(bin/codecept run Application -d -g AcceptanceTests-Job-${THISCHUNK} -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} "$@" --html reports.html)
                                                                            ^----------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
                                                                                                                                    ^---------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 677:
           COMMAND=(bin/codecept run Application -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} "$@" --html reports.html)
                                                                                                ^---------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 774:
               COMMAND=(bin/codecept run Application -d -g AcceptanceTests-Job-${THISCHUNK} -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} "$@" --html reports.html)
                                                                                ^----------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
                                                                                                                                        ^---------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 776:
               COMMAND=(bin/codecept run Application -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} "$@" --html reports.html)
                                                                                                    ^---------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 841:
               COMMAND="bin/codecept run Install -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} --html reports.html"
                ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 842:
               ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name ac-install-mariadb ${XDEBUG_MODE} -e XDEBUG_CONFIG="${XDEBUG_CONFIG}" ${CONTAINERPARAMS} ${IMAGE_PHP} ${COMMAND}
                                                                                                                                                                             ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 853:
               COMMAND="bin/codecept run Install -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} --html reports.html"
                ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 854:
               ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name ac-install-mysql ${XDEBUG_MODE} -e XDEBUG_CONFIG="${XDEBUG_CONFIG}" ${CONTAINERPARAMS} ${IMAGE_PHP} ${COMMAND}
                                                                                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 865:
               COMMAND="bin/codecept run Install -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} --html reports.html"
                ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 866:
               ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name ac-install-postgres ${XDEBUG_MODE} -e XDEBUG_CONFIG="${XDEBUG_CONFIG}" ${CONTAINERPARAMS} ${IMAGE_PHP} ${COMMAND}
                                                                                                                                                                              ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 877:
               COMMAND="bin/codecept run Install -d -c typo3/sysext/core/Tests/codeception.yml ${CODECEPION_ENV} --html reports.html"
                ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 878:
               ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name ac-install-sqlite ${XDEBUG_MODE} -e XDEBUG_CONFIG="${XDEBUG_CONFIG}" ${CONTAINERPARAMS} ${IMAGE_PHP} ${COMMAND}
                                                                                                                                                                            ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 904:
       COMMAND="cd Build; npm install && npm run build"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 905:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name build-css-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS} /bin/sh -c "${COMMAND}"
                                                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 913:
       COMMAND="php -dxdebug.mode=off bin/php-cs-fixer fix -v ${CGLCHECK_DRY_RUN} --config=Build/php-cs-fixer/config.php"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 914:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name cgl-${SUFFIX} ${IMAGE_PHP} ${COMMAND}
                                                                                          ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 926:
       COMMAND="php -dxdebug.mode=off bin/php-cs-fixer fix -v ${CGLCHECK_DRY_RUN} --config=Build/php-cs-fixer/header-comment.php"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 927:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name cgl-header-${SUFFIX} ${IMAGE_PHP} ${COMMAND}
                                                                                                 ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 963:
       COMMAND="if [ \$(git submodule status 2>&1 | wc -l) -ne 0 ]; then echo \"Found a submodule definition in repository\"; exit 1; fi"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 964:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name check-git-submodule-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                      ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 968:
       COMMAND="find 'typo3/sysext' -name '*.js' -not -path '*/Fixtures/*' -exec rm '{}' + && cd Build; npm ci || exit 1; node_modules/grunt/bin/grunt build; cd ..; git add *; git status; git status | grep -q \"nothing to commit, working tree clean\""
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 969:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name check-grunt-clean-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS} /bin/sh -c "${COMMAND}"
                                                                                                                                                   ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 973:
       COMMAND="git checkout -- composer.json; git checkout -- composer.lock; php -dxdebug.mode=off Build/Scripts/updateIsoDatabase.php; git add *; git status; git status | grep -q \"nothing to commit, working tree clean\""
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 974:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name check-iso-database-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                     ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 978:
       COMMAND="git checkout -- composer.json; git checkout -- composer.lock; php -dxdebug.mode=off Build/Scripts/updateCharsetTables.php; git add *; git status; git status | grep -q \"nothing to commit, working tree clean\""
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 979:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name check-charsets-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                 ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1050:
       COMMAND="composer config --unset platform.php; composer update --no-progress --no-interaction; composer dumpautoload"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1051:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name composer-install-max-${SUFFIX} -e COMPOSER_CACHE_DIR=.cache/composer -e COMPOSER_ROOT_VERSION=${COMPOSER_ROOT_VERSION} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                                                                                                               ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1055:
       COMMAND="composer config platform.php ${PHP_VERSION}.0; composer update --prefer-lowest --no-progress --no-interaction; composer dumpautoload"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1056:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name composer-install-min-${SUFFIX} -e COMPOSER_CACHE_DIR=.cache/composer -e COMPOSER_ROOT_VERSION=${COMPOSER_ROOT_VERSION} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                                                                                                               ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1060:
       COMMAND="cd Build/composer; rm -rf composer.json composer.lock public/index.php public/typo3 public/typo3conf/ext var/ vendor/; cp composer.dist.json composer.json; composer update --no-progress --no-interaction"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1061:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name composer-test-distribution-${SUFFIX} -e COMPOSER_CACHE_DIR=${CORE_ROOT}/.cache/composer -e COMPOSER_ROOT_VERSION=${COMPOSER_ROOT_VERSION} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                                                                                                                                  ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1071:
           COMMAND=(bin/phpunit -c Build/phpunit/FunctionalTests-Job-${THISCHUNK}.xml --exclude-group not-${DBMS} "$@")
                                                                      ^----------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
                                                                                                           ^-----^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 1073:
           COMMAND=(bin/phpunit -c Build/phpunit/FunctionalTests.xml --exclude-group not-${DBMS} "$@")
                                                                                          ^-----^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 1118:
       COMMAND="cd Build; npm ci || exit 1; node_modules/grunt/bin/grunt exec:lintspaces"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1119:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-html-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS} /bin/sh -c "${COMMAND}"
                                                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1123:
       COMMAND="php -v | grep '^PHP'; find typo3/ -name \\*.php -print0 | xargs -0 -n1 -P"'$(nproc 2>/dev/null || echo 4)'" php -dxdebug.mode=off -l >/dev/null"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.
                                                                                           ^-- SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.


In /project/Build/Scripts/runTests.sh line 1124:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-php-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1128:
       COMMAND="cd Build; npm ci || exit 1; node_modules/grunt/bin/grunt stylelint"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1129:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-css-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS} /bin/sh -c "${COMMAND}"
                                                                                                                                          ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1133:
       COMMAND="php -v | grep '^PHP'; find typo3/ -name 'Services.yaml' | xargs -r php -dxdebug.mode=off bin/yaml-lint --parse-tags"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1134:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-php-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1138:
       COMMAND="cd Build; npm ci || exit 1; node_modules/grunt/bin/grunt eslint"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1139:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-typescript-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS} /bin/sh -c "${COMMAND}"
                                                                                                                                                 ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1144:
       COMMAND="php -v | grep '^PHP'; find typo3/ \\( -name '*.yaml' -o -name '*.yml' \\) ! -name 'Services.yaml' | xargs -r php -dxdebug.mode=off bin/yaml-lint --no-parse-tags ${EXCLUDE_INVALID_FIXTURE_YAML_FILES}"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1145:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name lint-php-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                           ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1154:
       COMMAND=(php -dxdebug.mode=off bin/phpstan analyse -c Build/phpstan/${PHPSTAN_CONFIG_FILE} --verbose --no-progress --no-interaction --memory-limit 4G "$@")
                                                                            ^--------------------^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In /project/Build/Scripts/runTests.sh line 1159:
       COMMAND="php -dxdebug.mode=off bin/phpstan analyse -c Build/phpstan/${PHPSTAN_CONFIG_FILE} --verbose --no-progress --no-interaction --memory-limit 4G --generate-baseline=Build/phpstan/phpstan-baseline.neon"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1160:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name phpstan-baseline-${SUFFIX} ${IMAGE_PHP} /bin/sh -c "${COMMAND}"
                                                                                                                   ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.


In /project/Build/Scripts/runTests.sh line 1168:
       COMMAND="cd Build; npm ci || exit 1; CHROME_SANDBOX=false BROWSERS=chrome npm run test"
        ^-----^ SC2178 (warning): Variable was used as an array but is now assigned a string.


In /project/Build/Scripts/runTests.sh line 1169:
       ${CONTAINER_BIN} run ${CONTAINER_COMMON_PARAMS} --name unit-javascript-${SUFFIX} -e HOME=${CORE_ROOT}/.cache ${IMAGE_NODEJS_CHROME} /bin/sh -c "${COMMAND}"
                                                                                                                                                        ^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.

For more information:
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
  https://www.shellcheck.net/wiki/SC2128 -- Expanding an array without an ind...
  https://www.shellcheck.net/wiki/SC2178 -- Variable was used as an array but...

I will create a forge issue for every error class mentioned here: https://forge.typo3.org/issues/107967 https://forge.typo3.org/issues/107968 https://forge.typo3.org/issues/107969

Starkmann avatar Nov 03 '25 11:11 Starkmann

FYI - We declined that shellsheck stuff multiple times in the core already.

sbuerk avatar Nov 03 '25 12:11 sbuerk

I know, don't want to integrate shellcheck, i just want to apply some rules of shellcheck, we than can decide for every rule separately. Or has this done before already?

Example: We might apply SC2046 but skip SC2128

Starkmann avatar Nov 03 '25 12:11 Starkmann

SC2178 (warning): Variable was used as an array but is now assigned a string.

Sure, that is the intention and not a mistake. This has a couple of compat reasons, shellcheck works on latest shell versions and not of a broader support base. For example, default bash on macs are still outdated and even if some devs has newer versions installed we need the default version compat.

^--------^ SC2128 (warning): Expanding an array without an index only gives the first element.

Also done by intention, for the same reason as above.

And so on. Basically they are mostly warnings and "good" in many areas, but not suitable for that file.

You can do what you want here, but for core I'd bet that nothing will be done and declined. We have reason not to trust/use shellcheck .. neither in pipelines nor respect it because it collides with intentional stuff.

SC2046 may be an option .. but in the end not worth the time to provide, review and test. The places are save and well tested.

sbuerk avatar Nov 03 '25 12:11 sbuerk

I see. Our intention was to be as near to the core runTest.sh as possible, to make it easier to keep them in sync.

Seems like we have to discuss this here internally. Before i do a bunch off pull request nobody wants to have.

Starkmann avatar Nov 03 '25 12:11 Starkmann

I propose we add a full list of warning types here and then get @sbuerk's feedback on which of those would make sense to fix in the Core runTests.sh.

We should focus at whether we'd have these issues fixed in the Core first. I suspect that some changes were declined in the past because that particular solution had downsides, and not because the Core runTests.sh needs to keep a particular structure.

oliverklee avatar Nov 03 '25 12:11 oliverklee

We had call about this topic.

Discussion: We ignore SC2046 and SC2128 in our checks, and revert the related fixes. We try to integrate SC2178 in core because it is more readable and type save.

Starkmann avatar Nov 03 '25 14:11 Starkmann