-
Couldn't load subscription status.
- Fork 8
Add in various test GitHub Action workflows #49
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
Conversation
…eded dependencies for this new config. Ensure we use the same version of WordPress across all files
…tup proper config file
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…d the plugin from the current branch, not trunk
| - name: Checkout code | ||
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 | ||
| with: | ||
| ref: ${{ github.ref }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think with this in place the zip we build is always from trunk instead of the branch. Can't test that without merging this in due to this PR being opened from a fork but if you download the artifact from this PR, you'll see none of the changes made here reflected in that (and this breaks our Playground integration as we had a requirement of WP 6.9, which isn't out yet)
| data: { | ||
| resource: 'url', | ||
| url: `/plugin-proxy.php?org=WordPress&repo=ai&workflow=Build%20Plugin%20Zip&artifact=ai&pr=${prNumber}`, | ||
| url: `/plugin-proxy.php?org=WordPress&repo=ai&workflow=Pull%20Request%20Comments&artifact=ai&pr=${prNumber}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we call our Build Plugin Zip workflow from within this Pull Request Comments workflow, looks like the workflow name that shows in the API is Pull Request Comments, so updating that to ensure our Playground integration will work (though will also require the change referenced here)
| permissions: | ||
| contents: read | ||
| timeout-minutes: 20 | ||
| if: false # Temporarily disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled for now as we have no JS files and so this workflow will fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be ok to enable unless we feel confident we'll enable once we get JS in here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern with enabling it is it causes this workflow to always fail even though nothing is actually wrong. I have this noted to come back to so no concern with forgetting to enable this but happy to turn this on now if we want
|
|
||
| ```php | ||
| add_filter( | ||
| 'ai_feature_enabled', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like at some point the name of this filter was changed and no longer allows you to disable individual features. Didn't look through commit/comment history but wondering if that was intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be more clear, you can still disable an individual feature using the filter ai_feature_{$this->id}_enabled. But ai_feature_enabled no longer exists, seems to have been replaced with ai_features_enabled (feature vs features) and that filter doesn't pass the individual feature to it, so it's a universal disablement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkotter indeed, the ai_feature_enabled filter was refactored to the features version and now lives in the loader, due to finding that having two different filters was not good enough.
| $modified_title = apply_filters( 'document_title_parts', $title_parts ); | ||
|
|
||
| $this->assertEquals( 'My Site [AI]', $modified_title['site'] ); | ||
| $this->assertEquals( 'My Site [AI] [AI]', $modified_title['site'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the test but not sure why the [AI] part gets added twice. Seems like a likely bug in our Example Feature. Need to look closer at that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a5c7acc. Features were being initialized twice when running tests, resulting in the feature being applied twice (and thus seeing the double [AI] label). Seems to just be an issue in the tests but if anyone else wants to take a look, may be worth a second set of eyes to ensure there isn't a bug in our feature loader that would allow features to be loaded twice in normal circumstances
Description of the Change
This PR adds in the following GitHub Action workflows:
Also adds in / updates all necessary config files for this workflows and adds in / updates all necessary dependencies.
In addition, there were a handful of failures the above workflows flagged that have now been fixed up, including some PHPCS issues, PHPStan issues and some failing PHPUnit tests.
And last, tries to fix a couple issues with the
Pull Request Commentsworkflow that was merged in #41. This is unfortunately hard to test until these files are merged in as GitHub won't run these workflows from a fork it seems. But I'm hoping these are the last couple changes required there.Note
The above workflows were all copied from either the Abilities API repo or the MCP Adapter repo. I modified them slightly to work here but for the most part, left things alone. So if there are any changes we would like to make to the workflows or to the configs they use, that can be discussed.
How to test the Change
Ensure all workflows are running and passing on this PR
Test using WordPress Playground
The changes in this pull request can be previewed and tested using this WordPress Playground instance:
Click here to test this pull request.