From 2a47cb96317808e89d28eaca2c10549e00bf192f Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Thu, 31 Mar 2022 17:48:20 +0200
Subject: [PATCH] [TASK] runTests.sh: Remove .csv integrity handling
When core functional tests started to heavily rely
on CSV based import- and assertion files, we found
that editing such .csv files in Microsoft Excel leads
to warnings if the number of columns is not identical
for each row.
Script checkIntegrityCsvFixtures.php has then been
established to verify all rows of .csv fixture files
have the same amount of fields per file, and has been
enabled as CI job to ensure all existing fixture
files follow this.
Nowadays, this restriction feels archaic: Devs actively
working with these CSV files typically edit them in
an IDE like PhpStorm directly and don't use Excel for
this anymore. The PhpStorm plugin "Rainbox CSV" also
helps by coloring these files and other alternatives
like libreoffice do not have this 'all rows must have
same number of colums' restriction.
The patch drops the script, the runTests.sh usage and
the CI calls. This has the additional advantage that
line breaks for single fields are now possible, which
will further improve handling and readability of field
values in upcoming patches.
Resolves: #97274
Related: #83943
Releases: main, 11.5, 10.4
Change-Id: I2b4c2afc98c8471bccae1afb15e055182b563ee7
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/74131
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
Build/Scripts/checkIntegrityCsvFixtures.php | 239 ------------------
Build/Scripts/runTests.sh | 14 -
Build/gitlab-ci/nightly/integrity.yml | 1 -
Build/gitlab-ci/pre-merge/integrity.yml | 1 -
Build/testing-docker/local/docker-compose.yml | 28 --
5 files changed, 283 deletions(-)
delete mode 100755 Build/Scripts/checkIntegrityCsvFixtures.php
diff --git a/Build/Scripts/checkIntegrityCsvFixtures.php b/Build/Scripts/checkIntegrityCsvFixtures.php
deleted file mode 100755
index 9bc94e64796e..000000000000
--- a/Build/Scripts/checkIntegrityCsvFixtures.php
+++ /dev/null
@@ -1,239 +0,0 @@
-#!/usr/bin/env php
-<?php
-declare(strict_types=1);
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\CMS\Core\Utility\MathUtility;
-
-require __DIR__ . '/../../vendor/autoload.php';
-
-if (PHP_SAPI !== 'cli') {
- die('Script must be called from command line.' . chr(10));
-}
-
-/**
- * Core integrity test script:
- *
- * Find all CSV files in fixtures and make sure they have the correct column
- * count across all lines in them and fix them if --fix argument is given.
- */
-class checkIntegrityCsvFixtures
-{
- /**
- * @var bool True to fix broken files
- */
- private $fix = false;
-
- /**
- * @var bool True to drop superfluous comma on all CSV fixture files
- */
- private $fixAll = false;
-
- public function setFix(bool $fix)
- {
- $this->fix = $fix;
- }
-
- public function setFixAll(bool $fixAll)
- {
- $this->fixAll = $fixAll;
- }
-
- /**
- * Executes the CGL check.
- * The return value is used directly in the ext() call outside this class.
- *
- * @return int
- */
- public function execute(): int
- {
- $filesToProcess = $this->findCsvFixtures();
- $outputLines = [];
- $output = new \Symfony\Component\Console\Output\ConsoleOutput();
-
- $exitStatus = 0;
- /** @var \SplFileInfo $csvFixture */
- foreach ($filesToProcess as $csvFixture) {
- $fullFilePath = $csvFixture->getRealPath();
- if ($this->fixAll) {
- $changed = $this->fixCsvFile($fullFilePath);
- if ($changed) {
- $outputLines[] = 'Changed file "' . $this->formatOutputString($this->getRelativePath($fullFilePath)) . '"';
- }
- continue;
- }
- $singleFileScanResult = $this->validateCsvFile($fullFilePath);
- if ($singleFileScanResult !== '') {
- if ($this->fix) {
- $this->fixCsvFile($fullFilePath);
- $outputLines[] = 'Fixed file "' . $this->formatOutputString($this->getRelativePath($fullFilePath)) . '"';
- } else {
- $exitStatus = 1;
- $outputLines[] = 'File "' . $this->formatOutputString($this->getRelativePath($fullFilePath)) . '"'
- . ' is not in valid CSV format: ' . $singleFileScanResult;
- }
- }
- }
- if (!empty($outputLines)) {
- foreach ($outputLines as $line) {
- $output->writeln($line);
- }
- }
- return $exitStatus;
- }
-
- /**
- * Finds all CSV fixtures in TYPO3s core
- *
- * @return \Symfony\Component\Finder\Finder
- */
- private function findCsvFixtures(): \Symfony\Component\Finder\Finder
- {
- $finder = new Symfony\Component\Finder\Finder();
- $csvFixtures = $finder
- ->files()
- ->in(__DIR__ . '/../../typo3/sysext/*/Tests/Functional/**')
- ->name('*.csv');
- return $csvFixtures;
- }
-
- /**
- * Checks if a CSV uses the same amount of columns across all
- * lines in that file
- *
- * @param string $csvFixture
- * @return string
- */
- private function validateCsvFile(string $csvFixture): string
- {
- // Load file content into array split by line
- $lines = file($csvFixture);
- $columnCount = 0;
- foreach ($lines as $index => $line) {
- // count columns in file
- $columns = str_getcsv($line);
- if ($columnCount === 0) {
- $columnCount = count($columns);
- } else {
- if (count($columns) !== $columnCount) {
- // Skip CSV lines with starting with comments
- if (count($columns) === 1 && strpos($columns[0], '#') === 0) {
- continue;
- }
- return 'Line ' . ($index + 1) . '; Expected column count: ' . $columnCount . '; Actual: ' . count($columns);
- }
- }
- }
- return '';
- }
-
- /**
- * Fix a single CSV file.
- *
- * @param string $csvFixture
- * @return bool True if the file has been changed
- */
- private function fixCsvFile(string $csvFixture): bool
- {
- $changeNeeded = false;
- // Load file content into array split by line
- $lines = file($csvFixture);
- $neededColumns = 0;
- $csvLines = [];
- foreach ($lines as $line) {
- // Find out how many columns are needed in this file
- $csvLine = str_getcsv($line);
- $csvLines[] = $csvLine;
- foreach ($csvLine as $columnNumber => $columnContent) {
- if (!empty($columnContent) && $columnNumber + 1 > $neededColumns) {
- $neededColumns = $columnNumber + 1;
- }
- }
- }
- foreach ($csvLines as $csvLine) {
- // Set $changeNeeded to true if this file needs an update and line is not a comment
- if (count($csvLine) !== $neededColumns && substr($csvLine[0], 0, 2) !== '# ') {
- $changeNeeded = true;
- break;
- }
- }
- if ($changeNeeded) {
- // Update file
- $fileHandle = fopen($csvFixture, 'w');
- if (!$fileHandle) {
- throw new \Exception('Opening file "' . $csvFixture . '" for writing failed.');
- }
- foreach ($csvLines as $csvLine) {
- // Extend / reduce to needed size
- $csvLine = array_slice(array_pad($csvLine, $neededColumns, ''), 0, $neededColumns);
- $isComment = false;
- $line = array_reduce($csvLine, function ($carry, $column) use (&$isComment) {
- if ($carry === null && substr($column, 0, 2) === '# ') {
- $isComment = true;
- $carry .= $column;
- } elseif ($isComment) {
- // comment lines are not filled up with comma
- return $carry;
- } elseif (empty($column) && $column !== '0') {
- // No leading comma if first column
- $carry .= $carry === null ? '' : ',';
- } elseif (MathUtility::canBeInterpretedAsInteger($column)) {
- // No leading comma if first column and integer payload
- $carry .= ($carry === null ? '' : ',') . $column;
- } else {
- // No leading comma if first column and string payload and quote " to ""
- $column = str_replace('"', '""', $column);
- $carry .= ($carry === null ? '' : ',') . '"' . $column . '"';
- }
- return $carry;
- });
- fwrite($fileHandle, $line . chr(10));
- }
- fclose($fileHandle);
- }
- return $changeNeeded;
- }
-
- private function getRelativePath(string $fullPath): string
- {
- $pathSegment = str_replace('Build/Scripts', '', __DIR__);
- return str_replace($pathSegment, '', $fullPath);
- }
-
- /**
- * Makes the output on CLI a bit more readable
- *
- * @param string $filename
- * @return string
- */
- private function formatOutputString(string $filename): string
- {
- $pattern = '#typo3[\\\\/]sysext[\\\\/](?<extName>[a-z].+?)[\\\\/]Tests[\\\\/]Functional[\\\\/](?<file>.*)#';
- preg_match_all($pattern, $filename, $matches, PREG_SET_ORDER, 0);
- if (isset($matches[0])) {
- return 'EXT:' . $matches[0]['extName'] . ' > ' . $matches[0]['file'];
- }
- return $filename;
- }
-}
-
-$cglFixer = new checkIntegrityCsvFixtures();
-$args = getopt('', ['fix', 'fixAll']);
-if (array_key_exists('fix', $args)) {
- $cglFixer->setFix(true);
-}
-if (array_key_exists('fixAll', $args)) {
- $cglFixer->setFixAll(true);
-}
-exit($cglFixer->execute());
diff --git a/Build/Scripts/runTests.sh b/Build/Scripts/runTests.sh
index ee7be39e5699..1fc57c9a2881 100755
--- a/Build/Scripts/runTests.sh
+++ b/Build/Scripts/runTests.sh
@@ -96,7 +96,6 @@ Options:
- checkAnnotations: check php code for allowed annotations
- checkBom: check UTF-8 files do not contain BOM
- checkComposer: check composer.json files for version integrity
- - checkCsvFixtures: test integrity of functional test csv fixtures
- checkExceptionCodes: test core for duplicate exception codes
- checkExtensionScannerRst: test all .rst files referenced by extension scanner exist
- checkFilePathLength: test core file paths do not exceed maximum length
@@ -109,7 +108,6 @@ Options:
- composerInstallMin: "composer update --prefer-lowest", with platform.php set to PHP version x.x.0.
- composerValidate: "composer validate"
- docBlockCheck: Scan php doc blocks for validity
- - fixCsvFixtures: fix broken functional test csv fixtures
- functional: functional tests
- install: installation acceptance tests, only with -d mariadb|postgres|sqlite
- lint: PHP linting
@@ -493,12 +491,6 @@ case ${TEST_SUITE} in
SUITE_EXIT_CODE=$?
docker-compose down
;;
- checkCsvFixtures)
- setUpDockerComposeDotEnv
- docker-compose run check_csv_fixtures
- SUITE_EXIT_CODE=$?
- docker-compose down
- ;;
checkExceptionCodes)
setUpDockerComposeDotEnv
docker-compose run check_exception_codes
@@ -571,12 +563,6 @@ case ${TEST_SUITE} in
SUITE_EXIT_CODE=$?
docker-compose down
;;
- fixCsvFixtures)
- setUpDockerComposeDotEnv
- docker-compose run fix_csv_fixtures
- SUITE_EXIT_CODE=$?
- docker-compose down
- ;;
functional)
handleDbmsAndDriverOptions
setUpDockerComposeDotEnv
diff --git a/Build/gitlab-ci/nightly/integrity.yml b/Build/gitlab-ci/nightly/integrity.yml
index a3fc42a8ac79..4116beec83cb 100644
--- a/Build/gitlab-ci/nightly/integrity.yml
+++ b/Build/gitlab-ci/nightly/integrity.yml
@@ -57,7 +57,6 @@ integration various php 7.2:
- Build/Scripts/runTests.sh -s checkRst -p 7.2
- Build/Scripts/runTests.sh -s checkFilePathLength -p 7.2
- Build/Scripts/runTests.sh -s checkExtensionScannerRst -p 7.2
- - Build/Scripts/runTests.sh -s checkCsvFixtures -p 7.2
- Build/Scripts/runTests.sh -s checkBom -p 7.2
- Build/Scripts/runTests.sh -s checkComposer -p 7.2
diff --git a/Build/gitlab-ci/pre-merge/integrity.yml b/Build/gitlab-ci/pre-merge/integrity.yml
index ab17bd84b347..eb77dcc1ec17 100644
--- a/Build/gitlab-ci/pre-merge/integrity.yml
+++ b/Build/gitlab-ci/pre-merge/integrity.yml
@@ -45,7 +45,6 @@ integration various php 7.2 pre-merge:
- Build/Scripts/runTests.sh -s checkRst -p 7.2
- Build/Scripts/runTests.sh -s checkFilePathLength -p 7.2
- Build/Scripts/runTests.sh -s checkExtensionScannerRst -p 7.2
- - Build/Scripts/runTests.sh -s checkCsvFixtures -p 7.2
- Build/Scripts/runTests.sh -s checkBom -p 7.2
- Build/Scripts/runTests.sh -s checkComposer -p 7.2
diff --git a/Build/testing-docker/local/docker-compose.yml b/Build/testing-docker/local/docker-compose.yml
index 2dc691b064e4..047e6ffe7292 100644
--- a/Build/testing-docker/local/docker-compose.yml
+++ b/Build/testing-docker/local/docker-compose.yml
@@ -514,34 +514,6 @@ services:
php -dxdebug.mode=off Build/Scripts/checkIntegrityComposer.php;
"
- check_csv_fixtures:
- image: typo3/core-testing-${DOCKER_PHP_IMAGE}:latest
- user: "${HOST_UID}"
- volumes:
- - ${CORE_ROOT}:${CORE_ROOT}
- working_dir: ${CORE_ROOT}
- command: >
- /bin/sh -c "
- if [ ${SCRIPT_VERBOSE} -eq 1 ]; then
- set -x
- fi
- php -dxdebug.mode=off Build/Scripts/checkIntegrityCsvFixtures.php;
- "
-
- fix_csv_fixtures:
- image: typo3/core-testing-${DOCKER_PHP_IMAGE}:latest
- user: "${HOST_UID}"
- volumes:
- - ${CORE_ROOT}:${CORE_ROOT}
- working_dir: ${CORE_ROOT}
- command: >
- /bin/sh -c "
- if [ ${SCRIPT_VERBOSE} -eq 1 ]; then
- set -x
- fi
- php -dxdebug.mode=off Build/Scripts/checkIntegrityCsvFixtures.php --fix;
- "
-
check_exception_codes:
image: typo3/core-testing-${DOCKER_PHP_IMAGE}:latest
user: "${HOST_UID}"
--
GitLab