php icon indicating copy to clipboard operation
php copied to clipboard

Coordination: Upgrade to PHP 8.1 - 8.3 and PHPUnit 10

Open mk-mxp opened this issue 11 months ago • 3 comments

This is what I think must be done:

  • [x] Add PHP 8.3 to the CI of the track and allow it to fail #656
  • [x] Add PHPUnit 10 to the CI of the track and allow it to fail #656
  • [x] Fix the problems that occur with PHP 8.3 and PHPUnit 10
    • [x] Performance degrades when using PHP 8.3 #656
      • [x] XDEBUG_MODE=off solves this issue (XDebug is "more active" in default mode than before)
    • [x] Performance degrades when switching from PHPUnit 9 to 10
      • [x] XDEBUG_MODE=off required to get CI passing without timeouts (in #656)
      • [x] Timeouts for "robot-name" exercise during composer ci locally. It was not a problem at all with PHPUnit 9.6 (not even a noticable delay). XDEBUG_MODE=off doesn't help as much as it did for other exercises. Can be solved by replacing assertArrayNotHasKey($name, $names, ... with assertFalse(isset($names[$name]), .... This reduces runtime from minutes to milliseconds. Problem exists with PHPUnit >= 10.4 || < 11 #683
    • [x] Add a CI test to catch tests that would fail due to the runtime limit set by Exercism infrastructure. Uses timeout 54s <command> as a process based limit per exercise (3x 18 seconds, see forum for calculation details) #683
  • [x] Prepare the PHP test-runner to upgrade to PHP 8.3 / PHPUnit10. Use XDEBUG_MODE=off for performance reasons. https://github.com/exercism/php-test-runner/pull/105
  • [x] Prepare the PHP representer to upgrade to PHP 8.3. Use XDEBUG_MODE=off for performance reasons https://github.com/exercism/php-representer/issues/155
  • [x] Do the actual switch to PHP 8.1 - 8.3 and PHPUnit 10 as the production versions #704
    • [x] Merge test runner upgrade PR
    • [x] Update student facing documentation
    • [x] Remove PHP 8.0 / PHPUnit 9 from CI, add PHP8.1/8.2 to PHPUnit10 workflow

Post-upgrade to modernize further:

  • Merge #651 (requires PHP >= 8.1)
  • [ ] PHPUnit attributes, not annotations (annotations are deprecated, but still available)
    • [ ] Update test generator to produce attributes not annotations
    • [ ] Convert tests from annotations to attributes. Use rector/rector for that
  • [ ] Revert performance workaround in "robot-name" exercise (requires PHPUnit >= 11)

In the whole process, try to avoid re-running the representer and not to trigger re-testing all community solutions. These are very costly to Exercism.

mk-mxp avatar Mar 01 '24 07:03 mk-mxp

I can look into the first 3 and group them in one PR one of the next days.

The first 3 should be covered in https://github.com/exercism/php/pull/656 The PHP Test Runner PR https://github.com/exercism/php-test-runner/pull/100

The php-representer I don't know enough about to take care of. Update: I have looked a little into the PHP presenter, but as the tests even online, when looking into the logs are failing, it's hard to tell when the job is correctly done. So I would prefer someone with more knowledge about the presenter to handle this bit.

tomasnorre avatar Mar 03 '24 17:03 tomasnorre

I don't know if it belongs in this to-do list, but still some exercises, probably older ones, have snake cases in the Stub files. I would prefer having them using camel case too, like the standard is today.

E.g. the https://github.com/exercism/php/blob/main/exercises/concept/lasagna/Lasagna.php

tomasnorre avatar Mar 05 '24 08:03 tomasnorre

I haven't written documentation on this, yet: We use snake_case for functions, camelCase for methods and PascalCase for classes.

Edit: There is no "standard" defined for variable names, yet. And you may make this a separate issue, has nothing to do with PHP updates.

mk-mxp avatar Mar 05 '24 08:03 mk-mxp