Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run PHPUnit on PHP >= 7.0 #86

Merged
merged 10 commits into from
Mar 22, 2024
Merged

Run PHPUnit on PHP >= 7.0 #86

merged 10 commits into from
Mar 22, 2024

Conversation

reimic
Copy link
Collaborator

@reimic reimic commented Mar 21, 2024

What does this PR do?

  • refactors GitHub's actions to run a matrix of tests for PHP >= 7.0
  • implements PHPUnit-Polyfills
  • reorganizes test files for ease of access and extension
  • simplifies composers, exposing one slick file for library users
  • separates concerns, grouping dev-related files in the dev directory

What problem does it fix?

  • tests are now run for all required PHP versions
  • there were 2 top-level composer files, now only the right one is exposed

How to test if it works?

  • create a branch and set up a PR, the CI should run and test your code against all PHP & OS versions
  • expect at least 27 checks

@reimic
Copy link
Collaborator Author

reimic commented Mar 21, 2024

@adamziel Copying the CI setup from WordPress Core has proven itself to be much more difficult than we initially expected. At one point I had a bunch of code from a bunch of places throwing different errors for different PHP versions for different runs. I had to cut most of it out. Now I am adding it back again, but bit by bit, tackling each error one at a time.

But, in the meantime, we can discuss some aspects of the PR. Namely:

Separating concerns

Using the lib is distinctly different from contributing to it.

For users - we could offer a composer.json and a user-oriented README at root level. This could work well in tandem with a well-thought-out examples directory.

For contributors - we could be centered around the dev (developing? development? contributing?) directory. With a separate composer.json, contributor-oriented README. I believe Rector and coding standards would also go here. I am not certain about the proper home for model (re-)generation... and I think this comes from not knowing how users will interact with the lib. I'd also like to pick your brain on that.

(Actually, that is a topic robust enough, so I'll leave my questions at that.)

@adamziel
Copy link
Collaborator

For users - we could offer a composer.json and a user-oriented README at root level. This could work well in tandem with a well-thought-out examples directory.

Agreed. That composer.json would only contain some metadata and no actual dependencies.

For contributors - we could be centered around the dev (developing? development? contributing?) directory. With a separate composer.json, contributor-oriented README.

Agreed

I am not certain about the proper home for model (re-)generation.

bin/regenerate_models.php should be okay for now. We can always move it later.

and I think this comes from not knowing how users will interact with the lib. I'd also like to pick your brain on that.

Primarily via a CLI script like blueprints.php that is not yet present in this repo. Also, this will be published as a composer package so developers will reference it in their composer.json and then be able to use the public API – which isn't well-defined yet. For sure some version of run_blueprint($json) will be a part of it.

@adamziel
Copy link
Collaborator

This seems problematic – it installs PHPUnit 10, which is PHP8.1+, on PHP7:

CleanShot 2024-03-21 at 23 18 14@2x

@adamziel
Copy link
Collaborator

I wonder if the remaining PHP 7.0 error is with the ZIP decoder, not the test setup. It should be fine to comment out those tests for now and address the issue separately.

@reimic
Copy link
Collaborator Author

reimic commented Mar 22, 2024

Sure. I'll circle back to the Zip tests later.

So, what's enough for a merge here?
I am working on dev dependencies and adding CI steps. But this can be done elsewhere, too.

@reimic
Copy link
Collaborator Author

reimic commented Mar 22, 2024

So, it is not the stuff of legends after all...

@adamziel
Copy link
Collaborator

This is splendid @reimic! Thank you so much for all your hard work and dedication here, this one was a tough nut to crack and you did it!

@adamziel adamziel merged commit 228977b into WordPress:trunk Mar 22, 2024
27 checks passed
@adamziel adamziel changed the title Restructure workflows, tests and dev packages PHPUnit for PHP >= 7.0 Mar 22, 2024
@adamziel adamziel changed the title PHPUnit for PHP >= 7.0 Run PHPUnit on PHP >= 7.0 Mar 22, 2024
@adamziel adamziel mentioned this pull request Mar 22, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants