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.

Sign up for GitHub

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

Jump to bottom

PHP unit tests: remove WP_RUN_CORE_TESTS const #65631

Merged
merged 1 commit into from
Sep 25, 2024
Merged

PHP unit tests: remove WP_RUN_CORE_TESTS const #65631

merged 1 commit into from
Sep 25, 2024

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 25, 2024 •
edited
Loading

A recent update in Core caused the php tests to fail.

I asked about it here: https://core.trac.wordpress.org/ticket/61530#comment:54

I got an answer here: https://core.trac.wordpress.org/ticket/61530#comment:55

Removing the WP_RUN_CORE_TESTS constant added in #51950 will negate the check for IMPORTER_PLUGIN_FOR_TESTS, which is required for Core tests, but not Gutenberg (right now)

Testing Instructions

Stare at the PHP tests in CI and will for them to pass. 🤞🏻

@ramonjd
A recent update in core caused the php tests to fail. Removing this c…
aa0da15 
…onstant will negate the check for IMPORTER_PLUGIN_FOR_TESTS, which is required for core tests, but not Gutenberg (right now)

See: https://core.trac.wordpress.org/ticket/61530#comment:55
@ramonjd ramonjd self-assigned this Sep 25, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 25, 2024 •
edited
Loading

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: noisysocks <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ramonjd ramonjd added [Type] Build Tooling Issues or PRs related to build tooling No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core labels Sep 25, 2024
andrewserong
andrewserong approved these changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like the right fix to me! If the PHP tests pass, let's merge it.

If not, I suppose the two other options would be to see what else needs changing in GB in order for us to remove this define, or otherwise install the importer plugin within the GB tests.

Here's hoping this does the trick! 🤞

@ramonjd
Copy link
Member Author

ramonjd commented Sep 25, 2024

I could watch this all day:

2024-09-25 15 10 42

@ramonjd ramonjd enabled auto-merge (squash) September 25, 2024 05:12
@ramonjd ramonjd mentioned this pull request Sep 25, 2024
Merged
@noisysocks
Copy link
Member

noisysocks commented Sep 25, 2024 •
edited
Loading

I added the constant in #51950 to fix failing unit tests and said at the time:

A lot of these tests, I think, don't belong in Gutenberg anymore. They should have been removed when the functionality that they test were moved from Gutenberg to Core. It doesn't make sense to have tests in Gutenberg that test Core functionality.
...
I think setting WP_RUN_CORE_TESTS is all that we can do until these tests are removed.

It seems likely that this has happened since then.

So 👍 lgtm.

@ramonjd ramonjd merged commit e94673a into trunk Sep 25, 2024
69 of 72 checks passed
@ramonjd ramonjd deleted the try/fix-php-unit-tests-in-bootstrap branch September 25, 2024 05:39
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 25, 2024
@noisysocks noisysocks added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 30, 2024
@github-actions github-actions bot removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 30, 2024
gutenbergplugin pushed a commit that referenced this pull request Sep 30, 2024
@ramonjd @andrewserong @gutenbergplugin
PHP unit tests: remove WP_RUN_CORE_TESTS const (#65631)
4ab648d 
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: andrewserong <[email protected]>
@github-actions github-actions bot added the Backported to WP Core Pull request that has been successfully merged into WP Core label Sep 30, 2024
@github-actions GitHub Actions
Copy link

I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 4ab648d

@noisysocks noisysocks mentioned this pull request Sep 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewserong andrewserong andrewserong approved these changes

@noisysocks noisysocks Awaiting requested review from noisysocks

Assignees

@ramonjd ramonjd

Labels
Backported to WP Core Pull request that has been successfully merged into WP Core No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Milestone
Gutenberg 19.4
Development

Successfully merging this pull request may close these issues.
3 participants
@ramonjd @noisysocks @andrewserong