fix: repair CI pipeline (testing-framework, test paths, matrix, DI, CGL)#3
fix: repair CI pipeline (testing-framework, test paths, matrix, DI, CGL)#3
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request provides a comprehensive set of fixes to resolve continuous integration failures. It updates dependency constraints, modernizes test configurations for compatibility with newer TYPO3 and PHP versions, refactors test setup to use dependency injection, and applies various code style improvements to align with coding guidelines. The changes ensure the CI pipeline remains robust and functional across the supported TYPO3 and PHP environments. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive set of fixes for the CI pipeline, including updating dependencies, test paths, and the test matrix. It also refactors a test to use dependency injection and applies several coding style fixes. The changes are well-organized and accurately reflect the pull request description. I have one suggestion to improve the structure of a shell script for better readability and logic, but overall, the changes are solid.
CybotTM
force-pushed
the
fix/ci-repairs
branch
4 times, most recently
from
08f4209 to
215b2f3
Compare
CI StatusAll jobs pass except tests-main (v14) functional tests — these are pre-existing failures not caused by this PR:
Both issues exist on
Passing
|
CybotTM
force-pushed
the
fix/ci-repairs
branch
2 times, most recently
from
604c7ee to
2375476
Compare
CI Status — All GreenAll 30/30 jobs pass across all TYPO3 versions and PHP versions.
|
testing-framework 8.x requires typo3/cms-backend 12.*.*@dev || 13.*.*@dev, which conflicts with dev-main (now v14). testing-framework 9.x supports 13.*.*@dev || 14.*.*@dev. Allowing both ranges lets composer resolve the correct version based on the TYPO3 core version being tested. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The legacy path 'typo3conf/ext/codesnippet/Functional/Fixtures/...' cannot be resolved by testing-framework's linkTestExtensionsToInstance(). Use the composer package name 'typo3tests/example-extension' instead, which is resolved via getPackageInfoWithFallback() using the path repository already configured in composer.json. Also widen example_extension ext_emconf.php TYPO3 constraint to 12.4-14.x. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
- Update PHP versions per TYPO3 version: - v12.4: PHP 8.1, 8.2, 8.3, 8.4 - v13.4: PHP 8.2, 8.3, 8.4, 8.5 - v14/main: PHP 8.2, 8.3, 8.4, 8.5 - Replace tests-13.1 workflow with tests-13.4 (current LTS) - Remove dead 13.0/13.1 code from runTests.sh - Add PHP 8.4/8.5 support to runTests.sh - Restrict push trigger to main branch to avoid duplicate CI runs on PRs - Remove archived XLIFF linter job (TYPO3-CI-Xliff-Lint is archived, Docker image at container.registry.gitlab.typo3.org returns 403) - Fix duplicate "Unit Tests" step name (now "Functional Tests") Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
PhpDomainRenderer requires ComponentFactory via constructor injection. GeneralUtility::makeInstance() does not perform DI, causing: ArgumentCountError: Too few arguments to function __construct(), 0 passed Use $this->get() from the functional test container instead. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
- modifier_keywords: reorder `readonly protected` to `protected readonly` - operator_linebreak: move `.` concatenation operator to start of next line - method_argument_space: fix indentation of multiline function arguments - trailing_comma_in_multiline: add missing trailing comma - nullable_type_declaration: use `?Type` instead of `Type|null` - types_spaces: remove space in catch union type `\Exception|\Throwable` - cast_spaces: add space after cast `(string) $var` Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…tCase PHPUnit 11 (from testing-framework v9) makes TestCase::__construct() final. Move extension loading config from constructor to property declarations, which is compatible with both PHPUnit 10 and 11. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The render() methods referenced undefined variables ($sourceFile, $nodes, $caption, etc.) instead of reading from the $config array parameter. This caused 16 PHPStan errors. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
PHPUnit 11 (from testing-framework v9) removed static access to assertion methods, but CGL enforces self:: call style. Both work at runtime. Add PHPStan ignoreErrors pattern to suppress the false positive. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The extractTypeFromReturnComment method split at the first space, which broke types like `array<string, string|array>`. Add bracket-depth-aware splitting that only splits at spaces outside angle brackets. Also replace LanguageService and PageArguments test fixtures with local fixture classes to avoid dependency on TYPO3 core docblock wording that changes between versions. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de> Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
There was a problem hiding this comment.
Pull request overview
This PR is a comprehensive CI/test infrastructure repair, fixing multiple interrelated issues that were causing CI failures: testing-framework compatibility for TYPO3 v14, broken functional test extension loading paths, outdated CI matrices, DI container usage in tests, and CGL style violations.
Changes:
- Widens
typo3/testing-frameworkconstraint to^8.0.9 || ^9.0for TYPO3 v14 (dev-main) compatibility, and replaces test fixture dependencies on TYPO3 core classes with self-contained example extension classes (PropertyExample,MethodExample) to avoid version-specific breakage - Updates all CI matrices (PHP 8.1–8.5, TYPO3 12.4/13.4/main), renames
tests-13.1.yml→tests-13.4.yml, addspush: branches: [main]to all workflows, removes the archived XLIFF linter job, and fixes the duplicate "Unit Tests" step name - Fixes code style violations (modifier order, nullable syntax, operator linebreak, trailing commas, cast spacing) and adds logic to handle generic type spacing normalization in return comment parsing
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
composer.json |
Widens typo3/testing-framework constraint to support v9 (TYPO3 v14) |
Tests/Functional/ExtensionTestCase.php |
Replaces constructor-based property merging with direct property declarations; uses composer package name for extension loading |
Tests/Functional/PhpDomainRendererTest.php |
Uses DI container ($this->get()) instead of GeneralUtility::makeInstance() |
Tests/Functional/Fixtures/config/PageArguments.php |
Points to new MethodExample fixture class |
Tests/Functional/Fixtures/config/LanguageService.php |
Points to new PropertyExample fixture class |
Tests/Functional/Fixtures/results/PageArguments.rst |
Updates expected RST output for MethodExample |
Tests/Functional/Fixtures/results/LanguageService.rst |
Updates expected RST output for PropertyExample |
Tests/Functional/Fixtures/Extensions/example_extension/ext_emconf.php |
Widens TYPO3 version constraint to 12.4.0-14.99.99 |
Tests/Functional/Fixtures/Extensions/example_extension/Classes/PropertyExample.php |
New self-contained test fixture class replacing LanguageService |
Tests/Functional/Fixtures/Extensions/example_extension/Classes/MethodExample.php |
New self-contained test fixture class replacing PageArguments |
Classes/Domain/Factory/MethodFactory.php |
Adds splitTypeAndDescription and normalizeGenericTypeSpacing for correct generic type parsing; misc CGL fix |
Classes/Domain/Factory/PropertyFactory.php |
CGL fix: nullable return type syntax |
Classes/Domain/Factory/MemberFactory.php |
CGL fix: nullable return type syntax |
Classes/Command/PhpDomainCommand.php |
CGL fix: modifier order + cast spacing |
Classes/Command/BaselineCommand.php |
CGL fix: modifier order |
Classes/Renderer/CodeSnippetRenderer.php |
CGL fix: string concatenation operator placement |
Classes/Renderer/YamlCodeSnippetRenderer.php |
Refactors render method to read from $config array |
Classes/Renderer/XmlCodeSnippetRenderer.php |
Refactors render method to read from $config array |
Classes/Renderer/Traits/GetCodeBlockRstTrait.php |
CGL fix: indentation |
Classes/Util/CodeSnippetCreator.php |
CGL fix: trailing comma |
Build/phpstan/phpstan.neon |
Suppresses PHPStan false positives for static PHPUnit assertion methods |
Build/Scripts/runTests.sh |
Adds PHP 8.4/8.5 support, replaces 13.0/13.1 with 13.4, uses elif chain |
.github/workflows/tests-12.4.yml |
Adds push: branches: [main]; adds PHP 8.1 and 8.4; fixes duplicate step name |
.github/workflows/tests-13.4.yml |
Replaces old 13.1 workflow; adds PHP 8.4/8.5; fixes duplicate step name |
.github/workflows/tests-main.yml |
Adds push: branches: [main]; adds PHP 8.4/8.5; fixes duplicate step name |
.github/workflows/cgl.yml |
Updates matrix to 13.4/main; adds PHP 8.4/8.5; removes XLIFF linter job |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| return preg_replace_callback('/<[^>]+>/', static function (array $match): string { | ||
| return str_replace(', ', ',', $match[0]); | ||
| }, $type); |
There was a problem hiding this comment.
preg_replace_callback returns string|null (it returns null on PCRE error), but normalizeGenericTypeSpacing declares a string return type. While the pattern /<[^>]+>/ is valid and unlikely to trigger a PCRE error in practice, the return value should be safely handled to satisfy the declared return type. Wrapping the result with ?? $type (i.e., returning the original string as fallback if preg_replace_callback unexpectedly returns null) would make this type-safe.
| }, $type); | |
| }, $type) ?? $type; |
Summary
Comprehensive fix for all CI failures in a series of clean, atomic commits:
fix: allow testing-framework ^9.0— Widens thetypo3/testing-frameworkconstraint from^8.0.9to^8.0.9 || ^9.0, enabling compatibility with TYPO3 v14 (dev-main)fix: use composer package name for test extension loading— Replaces the legacytypo3conf/ext/path format with the composer package nametypo3tests/example-extensioninExtensionTestCase.php; widensext_emconf.phpTYPO3 constraint to12.4.0-14.99.99chore: update test matrix— Aligns workflow matrices with current TYPO3/PHP support:push: branches: [main]to avoid duplicate CI runs on PRsrunTests.shfix: use DI container in PhpDomainRendererTest— ReplacesGeneralUtility::makeInstance()with$this->get()so constructor-injected dependencies are resolvedstyle: fix CGL violations— Fixes 8 files flagged by php-cs-fixer: modifier order, nullable syntax, operator linebreak, trailing commas, cast spacingSupersedes
Test plan
PhpDomainRenderervia DI container