Fix pre-existing CI breakage: phpcs violations + phantom functional-tests testsuite - #13
Merged
Merged
Conversation
Trailing whitespace in language/de/overview.php, and a missing blank line after the namespace declaration in ext.php and textreparser/plugins/article_text.php, have been failing every CI run on main regardless of what a given commit actually changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ctory phpunit.xml.dist declares an "Extension Functional Tests" testsuite covering ./tests/functional/, but that directory has never existed in this extension - only tests/migrations/ does. The shared CI workflow's RUN_FUNCTIONAL_TESTS: 0 only adds --exclude-group functional, which filters annotated tests but still requires every declared <directory> to exist, so PHPUnit fails immediately with "Test directory ... not found" before any test runs, on every DB/PHP matrix job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ion succeeds The reusable test-framework workflow generates each DB matrix job's phpunit config by copying phpBB core's own .github/phpunit-*-github.xml template, which always declares a "phpBB Functional Tests" testsuite pointing at ../tests/functional. RUN_FUNCTIONAL_TESTS: 0 only adds --exclude-group functional (a test-group filter), it does not skip declaring that directory, so PHPUnit still requires it to exist before any test runs. Confirmed by reproducing the workflow's exact sed-based config generation locally and tracing the "Test directory ... not found" error to this. Other phpbbmodders extensions without functional tests instead disable the entire DB matrix (RUN_MYSQL_JOBS: 0, etc.), but this extension has a real DB-backed migration test (tests/migrations/database/ add_article_description_test.php) worth actually running, so keeping the DB matrix enabled and adding this empty placeholder directory preserves that coverage instead of losing it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
language/de/overview.phpand a missing blank line after the namespace declaration inext.phpandtextreparser/plugins/article_text.php. These were failing thephpcsstyle-check job on every push tomain, blocking the whole DB test matrix from ever running.phpunit.xml.dist's "Extension Functional Tests" testsuite, which declared./tests/functional/— a directory that never existed in this extension. This is the config used for local dev, not by the CI matrix jobs, but it was inaccurate either way.tests/functional/.gitkeep. The reusable CI workflow generates each DB matrix job's actual phpunit config by copying phpBB core's own.github/phpunit-*-github.xmltemplate, which always declares a testsuite pointing at../tests/functional.RUN_FUNCTIONAL_TESTS: 0only adds--exclude-group functional(a test-group filter) — it doesn't skip declaring that directory, so PHPUnit still requires it to exist before any test runs. Confirmed by reproducing the workflow's exact config-generation step locally and tracing the "Test directory ... not found" error to this. Other phpbbmodders extensions without functional tests instead disable the whole DB matrix (RUN_MYSQL_JOBS: 0, etc.), but this extension has a real DB-backed migration test worth actually running, so this keeps that coverage instead of losing it.All three are pre-existing and unrelated to any pending feature/docs PR's own content — confirmed by checking
main's own CI history, which has been failing on the phpcs step for several commits already.Test plan
php -lon all three phpcs-fixed files: no syntax errors.phpcscommand CI uses (--standard=build/code_sniffer/ruleset-php-extensions.xml) against the whole extension: clean, no violations.backupGlobalsincompatibility with this old phpunit.xml schema before reaching directory discovery), so CI itself is the real verification for the last two fixes.Investigated and written by Claude on behalf of William Jacoby (bonelifer).