Fix tests and CI#65
Conversation
📝 WalkthroughWalkthroughThis update revises the CI pipeline and test configuration. The CI workflow now expands the test matrix to cover more MediaWiki versions, improves caching with updated action versions, and adjusts test conditions and job properties. A new installation script automates MediaWiki setup including dependency installation and error reporting configuration. In addition, the PHPUnit configuration is streamlined by removing the “System” testsuite. Several tests are enhanced by adding group annotations, updating assertion checks, and refining method parameter types. The parser test suite also now reflects updated error message HTML structures and versioning. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/installMediaWiki.sh (1)
17-23: Consider using environment variables for sensitive data.The database credentials and admin password are hardcoded. Consider using environment variables for sensitive data.
+DB_USER="${DB_USER:-root}" +DB_NAME="${DB_NAME:-mw}" +ADMIN_PASS="${ADMIN_PASS:-AdminPassword}" + php maintenance/install.php \ --dbtype sqlite \ - --dbuser root \ - --dbname mw \ + --dbuser "$DB_USER" \ + --dbname "$DB_NAME" \ --dbpath "$(pwd)" \ --scriptpath="" \ - --pass AdminPassword WikiName AdminUser + --pass "$ADMIN_PASS" WikiName AdminUser
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/ci.yml(9 hunks).github/workflows/installMediaWiki.sh(1 hunks)phpunit.xml.dist(0 hunks)tests/Integration/BitbucketFunctionIntegrationTest.php(1 hunks)tests/Integration/EmbedFunctionIntegrationTest.php(2 hunks)tests/TestEnvironment.php(1 hunks)tests/Unit/Adapters/ParserFunctionEmbedPresenterTest.php(1 hunks)tests/parser/parserTests.txt(4 hunks)
💤 Files with no reviewable changes (1)
- phpunit.xml.dist
✅ Files skipped from review due to trivial changes (1)
- tests/Integration/BitbucketFunctionIntegrationTest.php
🔇 Additional comments (33)
tests/TestEnvironment.php (1)
24-24: LGTM! Improved type declaration.The change to use the explicit nullable type
?Titleinstead of the implicitTitle = nullmakes the code more type-safe and self-documenting.tests/Integration/EmbedFunctionIntegrationTest.php (2)
13-13: LGTM! Added database group annotation.The
@group Databaseannotation properly categorizes this test as requiring database access.
64-65: LGTM! Updated category assertions to use new API.The assertions have been correctly updated to use
getCategoryNames()instead ofgetCategories(), aligning with MediaWiki's API changes.Also applies to: 69-69
tests/Unit/Adapters/ParserFunctionEmbedPresenterTest.php (1)
40-47: LGTM! Enhanced error message assertions.The test now properly handles multiple error message formats across different MediaWiki versions using PHPUnit's built-in
assertThatwithlogicalOr.tests/parser/parserTests.txt (4)
1-1: Version Update VerificationThe version indicator has been updated to "Version 3". Please ensure that all related documentation, configurations, and test references are updated to reflect this new version.
32-32: Updated Error Message for Missing FileThe error markup for the embed function when a file is not found now follows the new HTML structure. Confirm that the frontend styling and any client-side processing correctly interpret this format.
42-42: Updated Error Message for Invalid Bitbucket URLThe HTML structure for the Bitbucket error message has been updated and appears consistent with the new design standards. Please verify that these changes integrate seamlessly with your overall error handling.
52-52: Updated Error Message for Disallowed File ExtensionThe error message for embedding files with disallowed extensions now uses the updated HTML format. Make sure that any integration tests or UI components that rely on this output are adjusted accordingly.
.github/workflows/ci.yml (25)
10-10: Continue-on-Error Configuration in Test JobThe
continue-on-error: ${{ matrix.experimental }}property now lets experimental matrix configurations bypass some failures. Double-check that this behavior is intentional to avoid masking genuine issues during CI runs.
15-33: Matrix Configuration for CI JobsThe test job’s matrix now includes multiple MediaWiki versions (REL1_39 through master) with corresponding PHP versions and an experimental flag. Please verify that the PHP version assignments and the use of the experimental flag precisely match the project's compatibility and testing objectives.
49-57: Updated MediaWiki Cache ActionThe workflow now uses
actions/cache@v4for caching the MediaWiki installation, with explicit exclusions for extensions and vendor directories. Confirm that these cache paths meet your build and performance expectations.
59-64: Updated Composer Cache ActionComposer caching has been updated to use
actions/cache@v4. This should help speed up dependency resolution; please ensure that the cache key (composer-php${{ matrix.php }}) remains unique per PHP version.
65-68: Usage of Checkout Action V4 for EarlyCopyThe repository is now checked out using
actions/checkout@v4into theEarlyCopydirectory. Verify that all subsequent steps referencing this path correctly locate the intended files.
69-73: Refactored MediaWiki Installation StepThe installation process now executes via the custom script (
installMediaWiki.sh) located in the EarlyCopy directory. Please check that the specified working directory (working-directory: ~) is appropriate and that the script completes successfully for all defined MediaWiki versions.
74-77: Extensions Repository CheckoutThe subsequent checkout for the ExternalContent extension (to
mediawiki/extensions/ExternalContent) appears correctly configured. Ensure that this path remains consistent with your project structure.
90-91: Conditional Execution for Code CoverageThe code coverage steps are now executed conditionally when
matrix.mw == 'master'. This is a good strategy to limit resource-intensive operations to the master branch, but please confirm that this is the desired behavior.
97-99: Parser Tests Conditional ExecutionThe parser tests are set to run only if
matrix.mwis greater than or equal to "REL1_43". Since this comparison is string-based, please verify that the lexicographical ordering behaves as intended across all matrix entries.
101-110: Static Analysis Job Matrix SetupThe static-analysis job targets MediaWiki version 'REL1_43' with PHP 8.3, which appears to be a streamlined configuration for code analysis. Confirm that this subset adequately covers your static analysis needs.
124-132: Static Analysis MediaWiki CacheThe cache setup for MediaWiki in the static-analysis job correctly uses
actions/cache@v4with the keymw_static_analysis. Ensure that this key remains distinct from other cache keys to prevent conflicts.
134-138: Static Analysis Composer CacheThe Composer cache configuration for static-analysis is updated to use version v4 with a unique cache key. This should help maintain performance consistency during the analysis.
140-143: Static Analysis Repository CheckoutRetrieving the repository into the
EarlyCopydirectory for the static-analysis job is consistent with the test job process. Please verify that the paths used throughout remain aligned.
144-148: Static Analysis MediaWiki InstallationSimilar to the test job, the static-analysis job installs MediaWiki using the custom installation script from the
EarlyCopydirectory. Confirm that the working directory configuration and script execution are functioning as expected.
149-152: Static Analysis Extensions CheckoutThe step checking out the ExternalContent extension for the static-analysis job appears correctly structured. No issues are detected.
159-161: PHPStan Analysis ConfigurationThe PHPStan step is configured to run with error formatting piped through
cs2pr. Verify that the PHPStan and cs2pr versions used are compatible and that the output meets your integration requirements.
162-163: Psalm Analysis ConfigurationThe Psalm step employs the
--shepherdand--statsoptions. Confirm these flags provide the desired level of detail for your static analysis reporting.
166-174: Code Style Job Matrix SetupThe code-style job now targets MediaWiki version 'REL1_43' with PHP 8.3. Validate that these settings are sufficient for enforcing your project’s coding standards.
189-197: Code Style MediaWiki CacheThe caching configuration for MediaWiki in the code-style job leverages
actions/cache@v4with a cache key consistent with static analysis. This setup looks solid—ensure that it aligns with your caching strategy.
199-204: Code Style Composer CacheThe Composer cache for the code-style job is updated similarly and uses a distinct cache key (
composer_static_analysis), which should help avoid collisions.
205-208: Code Style Repository CheckoutThe repository checkout into the
EarlyCopydirectory is repeated here for the code-style job. Please verify that this usage is consistent with subsequent path references.
209-213: Code Style MediaWiki InstallationThe installation of MediaWiki via the custom script in the EarlyCopy directory is now part of the code-style job as well. Ensure that the working-directory configuration (
~) is intended and that the installation process is robust across all conditions.
214-217: Code Style Extensions CheckoutThe step for checking out the ExternalContent extension is consistent with previous jobs. Confirm that the destination path remains valid.
218-223: Composer Configuration for Code StyleThe Composer configuration and installation steps in the code-style job are straightforward and follow best practices. No issues detected.
224-225: PHP CodeSniffer ExecutionRunning PHPCS with the
-p -sflags is an effective way to enforce and display coding standards. Ensure that the defined standards align with your project requirements.
| wget "https://github.com/wikimedia/mediawiki/archive/refs/heads/$MW_BRANCH.tar.gz" -nv | ||
|
|
||
| tar -zxf $MW_BRANCH.tar.gz | ||
| mv mediawiki-$MW_BRANCH mediawiki |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for download and extraction.
The script should check for successful download and extraction of MediaWiki.
-wget "https://github.com/wikimedia/mediawiki/archive/refs/heads/$MW_BRANCH.tar.gz" -nv
-
-tar -zxf $MW_BRANCH.tar.gz
-mv mediawiki-$MW_BRANCH mediawiki
+if ! wget "https://github.com/wikimedia/mediawiki/archive/refs/heads/$MW_BRANCH.tar.gz" -nv; then
+ echo "Failed to download MediaWiki"
+ exit 1
+fi
+
+if ! tar -zxf "$MW_BRANCH.tar.gz"; then
+ echo "Failed to extract MediaWiki"
+ exit 1
+fi
+mv "mediawiki-$MW_BRANCH" mediawiki📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| wget "https://github.com/wikimedia/mediawiki/archive/refs/heads/$MW_BRANCH.tar.gz" -nv | |
| tar -zxf $MW_BRANCH.tar.gz | |
| mv mediawiki-$MW_BRANCH mediawiki | |
| if ! wget "https://github.com/wikimedia/mediawiki/archive/refs/heads/$MW_BRANCH.tar.gz" -nv; then | |
| echo "Failed to download MediaWiki" | |
| exit 1 | |
| fi | |
| if ! tar -zxf "$MW_BRANCH.tar.gz"; then | |
| echo "Failed to extract MediaWiki" | |
| exit 1 | |
| fi | |
| mv "mediawiki-$MW_BRANCH" mediawiki |
| MW_BRANCH=$1 | ||
| EXTENSION_NAME=$2 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for command line arguments.
The script should validate that both required arguments are provided and non-empty.
-MW_BRANCH=$1
-EXTENSION_NAME=$2
+if [ -z "$1" ] || [ -z "$2" ]; then
+ echo "Usage: $0 <mw_branch> <extension_name>"
+ exit 1
+fi
+
+MW_BRANCH="$1"
+EXTENSION_NAME="$2"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MW_BRANCH=$1 | |
| EXTENSION_NAME=$2 | |
| if [ -z "$1" ] || [ -z "$2" ]; then | |
| echo "Usage: $0 <mw_branch> <extension_name>" | |
| exit 1 | |
| fi | |
| MW_BRANCH="$1" | |
| EXTENSION_NAME="$2" |
| echo '$wgShowDBErrorBacktrace = true;' >> LocalSettings.php | ||
| echo '$wgDevelopmentWarnings = true;' >> LocalSettings.php | ||
|
|
||
| echo 'wfLoadExtension( "'$EXTENSION_NAME'" );' >> LocalSettings.php |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use safer string interpolation for extension loading.
The current string concatenation could be unsafe if $EXTENSION_NAME contains special characters.
-echo 'wfLoadExtension( "'$EXTENSION_NAME'" );' >> LocalSettings.php
+printf 'wfLoadExtension( "%s" );\n' "$EXTENSION_NAME" >> LocalSettings.php📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo 'wfLoadExtension( "'$EXTENSION_NAME'" );' >> LocalSettings.php | |
| printf 'wfLoadExtension( "%s" );\n' "$EXTENSION_NAME" >> LocalSettings.php |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
102-103: Update Platform Requirements and Unordered List StyleThe updated platform requirements accurately reflect that the extension now supports PHP 8.0 or later (tested up to 8.3) and MediaWiki 1.35 or later (tested up to 1.43 and master). However, note that the unordered list items on these lines use asterisks (*) while other sections (e.g., line 15) use dashes (-) for list markers. For consistency with the markdown style guidelines (as also flagged by static analysis), please consider updating these lines to use dashes.
A potential diff for this change:
-* [PHP] 8.0 or later (tested up to 8.3) -* [MediaWiki] 1.35 or later (tested up to 1.43 and master) + - [PHP] 8.0 or later (tested up to 8.3) + - [MediaWiki] 1.35 or later (tested up to 1.43 and master)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
102-102: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
103-103: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
102-102: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
103-103: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
| * [PHP] 8.0 or later (tested up to 8.1) | ||
| * [MediaWiki] 1.35 or later (tested up to 1.39 and master) | ||
| * [PHP] 8.0 or later (tested up to 8.3) | ||
| * [MediaWiki] 1.35 or later (tested up to 1.43 and master) |
There was a problem hiding this comment.
If we're dropping CI for 1.35, then we should remove support for it completely. Otherwise, we have no way to verify that 1.35 works with any new changes (which is not worth spending time on since it's EOL anwyay).
3 participants
Key changes
Summary by CodeRabbit