Skip to content

Conversation

@stmh
Copy link
Member

@stmh stmh commented Nov 23, 2025

No description provided.

renovate bot and others added 4 commits November 21, 2025 09:10
- Update minimum PHP version from 8.2 to 8.3 (required by PHPUnit 12)
- Upgrade PHPUnit from 9.6.23 to 12.4.4
- Remove tests-8.2.yml workflow (no longer compatible)
- Update release.yml workflow to use PHP 8.3
- Update documentation to reflect PHP 8.3 requirement
- Update composer platform config to PHP 8.3

PHPUnit 12 requires PHP 8.3+ which necessitates these breaking changes.
- Replace assertContains() with assertStringContainsString() for string checks
- Replace deprecated setMethods() with onlyMethods() in mock builders
- Fix lowercase setup() methods to setUp(): void
- Replace deprecated returnValue() with willReturn()
- Update phpunit.xml schema to PHPUnit 12.0

These changes address the critical blocking issues identified in PR #354.
@claude
Copy link

claude bot commented Nov 23, 2025

Code Review - PHPUnit 12 Upgrade

Overview

This PR upgrades PHPUnit from version 9.6.23 to 12.4.4 and increases the minimum PHP requirement from 8.2 to 8.3. This is a necessary breaking change as PHPUnit 12 requires PHP 8.3+.

Positive Aspects ✅

  1. Comprehensive Test Updates: All 35+ test files have been systematically updated with PHPUnit 12 compatibility fixes:

    • assertContains()assertStringContainsString() for string assertions
    • setMethods()onlyMethods() in mock builders (deprecated method removed)
    • returnValue()willReturn() in mock expectations
    • setUp() methods properly typed with : void
  2. Proper Dependency Management:

    • PHPUnit and all related packages updated consistently (php-code-coverage, php-invoker, php-timer, etc.)
    • Dependencies properly constrained to compatible versions
    • composer.lock properly regenerated
  3. Documentation Updated: Installation docs correctly updated to reflect PHP 8.3 requirement

  4. CI/CD Alignment:

    • Release workflow updated to use PHP 8.3
    • Removed incompatible PHP 8.2 test workflow
    • Existing PHP 8.3 and 8.4 test workflows remain functional
  5. Clear Commit History: Well-structured commits with descriptive messages explaining the changes

Issues & Recommendations 🔍

Critical Issues

None identified - the upgrade appears complete and well-executed.

Minor Observations

  1. Breaking Change Communication: This is a major breaking change for users. Consider:

    • Adding a CHANGELOG.md entry highlighting the PHP 8.3 requirement
    • Bumping the major/minor version appropriately (semantic versioning)
    • Including upgrade instructions for users on PHP 8.2
  2. PHPUnit Configuration: The phpunit.xml was simplified to the minimal schema. While this works, consider if any of these removed settings were beneficial:

    • Code coverage configuration
    • Test execution options (stopOnFailure, etc.)
    • Logging/result output settings
  3. Test Coverage Verification:

    • Verify that no test functionality was lost in the migration
    • Consider running code coverage reports to ensure coverage hasn't decreased
  4. Deprecation Path: The onlyMethods() vs setMethods() change is good, but verify that:

    • All mock setups still work as intended
    • No methods are accidentally being mocked that shouldn't be

Security Considerations ✅

  • No security concerns identified
  • Dependencies updated from trusted sources (official PHPUnit packages)
  • No changes to authentication, authorization, or data handling

Testing Recommendations

Before merging, please verify:

  1. composer install completes successfully
  2. ./vendor/bin/phpunit tests --exclude-group docker passes all tests
  3. ./vendor/bin/grumphp run (quality checks) passes
  4. Build process (composer build-phar) works with new dependencies
  5. Consider testing on both PHP 8.3 and 8.4 environments

Performance Considerations

PHPUnit 12 includes performance improvements over version 9, particularly in:

  • Test discovery and execution
  • Mock object generation
  • Memory usage

This should result in faster test execution overall. ✅

Conclusion

This is a well-executed upgrade that properly handles all the breaking changes between PHPUnit 9 and 12. The test updates are comprehensive and follow PHPUnit 12 best practices.

Recommendation: ✅ Approve with minor suggestions

The main action item is ensuring users are properly informed about the PHP 8.3 requirement through release notes and version bumping.

Great work on the thorough migration! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants