Skip to content

Conversation

@hulkoba
Copy link

@hulkoba hulkoba commented Dec 3, 2025

Overview

This work is conducted in accordance with "Milestone 1: Use RectorPHP for upgrading PHPUnit tests," as outlined in the Scope of Work for the Sovereign Tech Agency Resilience Program.

Note: We open this PR on master branch. Please let us know if you want to have it elsewhere.

This PR:

  • adds initial rector setup
  • adds custom Rector rules for these patterns
  • adds documentation for Rector rules

Run rector

This requires files in the directory path in rector.php file

vendor/bin/rector process src --dry-run

Run rector test

vendor/bin/phpunit tests --filter RuleName

@terrafrost
Copy link
Member

I'll take a look this weekend - thanks!

@terrafrost
Copy link
Member

Give me like two more days on this. I had some unexpected things come up over the weekend.

Thanks!

@terrafrost
Copy link
Member

So ideally this would be in earlier versions of phpseclib. It's the earlier versions of phpseclib that run on a large gamut of PHP versions. Like although phpseclib 1.0 will run on both PHP 5.3 and PHP 8.5 I don't believe there's any version of PHPUnit that'll do the same, hence the need for tooling to automatically upgrade PHPUnit.

The master branch of phpseclib only works on PHP 8.2+. And who knows, maybe the minimum version of PHP will go up.

Like ideally rector would replace lines like this in .github/workflows/ci.yml (from the 1.0 branch):

-   name: Make Tests Compatiable With PHPUnit 9+
    if: contains(fromJSON('["7.3", "7.4", "8.0", "8.1", "8.2", "8.3", "8.4", "8.5"]'), matrix.php-version)
    run: php tests/make_compatible_with_phpunit9.php

It's not needed for the master branch because .github/workflows/ci.yml in the master branch doesn't do any php tests/make_compatible_with_phpunit9.php-like calls:

https://github.com/phpseclib/phpseclib/blob/master/.github/workflows/ci.yml

That said, in looking into it, now, I guess one problem with RectorPHP doing this is that it'd be running on older versions of PHP. Like I see that the latest version of RectorPHP requires PHP 7.4+ so we couldn't run that version on PHP 7.3 BUT we could do RectorPHP 0.8.9 on PHP 7.2. I'm not sure what the last version of RectorPHP was that worked on PHP 7.2 but Composer could figure that out.

I see that phpseclib 3.0 calls php tests/make_compatible_with_phpunit7.php for PHP 7.1+ and php tests/make_compatible_with_phpunit9.php for PHP 7.3+. It's strange that phpseclib 3.0 would need to do php tests/make_compatible_with_phpunit7.php but phpseclib 1.0 wouldn't. Like phpseclib 1.0 is unit tested on PHP 5.3+ whereas phpseclib 3.0 is only unit tested on PHP 7.0+ so why don't the unit tests for phpseclib 1.0 on PHP 7.1 need to be made compatible with PHPUnit 7 whereas they do for phpseclib 3.0? Maybe the key take away is that php tests/make_compatible_with_phpunit7.php isn't even necessary for phpseclib 3.0? idk. If it is absolutely necessary for the unit tests to work on PHP 7.1 with phpseclib 3.0 then I suppose we could just unit test phpseclib 3.0 on PHP 7.2+.

Unrelated to all of that... I would have thought you would have used https://github.com/rectorphp/rector-phpunit/ . Like instead of implementing a whole AddVoidLifecycleAssert class you could have just done something like this:

https://getrector.com/demo/b1390585-12e3-4f5d-9d4f-ad7f3312d937

Or am I missing something?

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