Conversation
t-hamano
changed the title
|
Size Change: 0 B Total Size: 2.45 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 1f64080. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19106714643
|
Contributor
Author
|
@aduth @youknowriad, the static checks on the trunk branch are unstable, and we are trying to resolve this. I updated Example:
|
Contributor
|
By default sass has his own resolution behavior when it comes to importing SASS from other packages. I know there's a way to configure it to use the Node.JS resolution and I think @aduth previously suggested that. I wonder if it solves that error if we switch to it (might require some changes in some CSS files). |
t-hamano
commented
Nov 5, 2025
.github/workflows/static-checks.yml
Outdated
Comment on lines
36
to
45
| # exclude: | ||
| # # On PRs: Only test Node 20 on Ubuntu | ||
| # - event: 'pull_request' | ||
| # node: '22' | ||
| # - event: 'pull_request' | ||
| # node: '24' | ||
| # - event: 'pull_request' | ||
| # os: 'macos-15' | ||
| # - event: 'pull_request' | ||
| # os: 'windows-2025' |
Contributor
Author
There was a problem hiding this comment.
If we are able to resolve the issue with this pull request, I will revert this commented-out code.
Member
|
I have several thoughts around that build failure:
|
This was referenced Nov 5, 2025
t-hamano
changed the title
Contributor
Author
I tried this approach. I'm not entirely confident, but it seems like the license check is now working correctly locally. |
aduth
approved these changes
Nov 5, 2025
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
That looks like a good solution 👍
packages/scripts/utils/license.js
Outdated
| // due to platform mismatch. If optional and not installed, skip with a warning. | ||
| const packageJsonPath = dep.path + '/package.json'; | ||
| if ( | ||
| process.platform === 'win32' && |
Member
There was a problem hiding this comment.
Do we need to be specific to the platform, or should this behavior apply broadly?
Contributor
Author
There was a problem hiding this comment.
I thought it would be best to check the licenses of the relevant libraries as much as possible, so for now I'm limiting it to specific platforms. However, I don't have a strong opinion on this.
Member
There was a problem hiding this comment.
I guess it's not clear to me if this is a Windows specific behavior, and that for me it makes sense to agree to an expectation that "if it's not installed" + "it's an optional dependency" = "we don't care to check". My worry would be if this leaves us open to similar issues in the future if, for example, an optional package isn't installed on macOS.
Contributor
Author
There was a problem hiding this comment.
I see, let's apply this check to all platforms.
But before that, I'm double-checking whether this check was really necessary: #72996 (comment)
Contributor
Author
|
Hmm, I thought a log entry saying "Skipping..." should be recorded on Windows CI, but nothing is being logged: 
I may have been mistaken about something. Perhaps simply updating the lock file would have been sufficient. Let's revert the changes to the script file and see what happens with the CI. |
aduth
approved these changes
Nov 5, 2025
Member
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
It looks like the build is happy with just the lockfile revisions 👍 Now just need to revert the workflow changes to test other platforms.
Contributor
Author
|
Yes, it's strange, but it might have been a temporary issue with the CI. For now, let's just update the lockfile and see what happens. |
t-hamano
changed the title
|
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. |
Member
|
I'm going to go ahead and merge this to resolve the issues present on |
Member
|
The "unable to locate" error surfaced again when this was merged to Maybe the issue is intermittent. |
Member
Trying another fix in #73026 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Attempt to fix the CI failures occurring on the trunk branch.
Note
For testing purposes, I'm temporarily running CI on all operating systems. If all CI checks pass, I will revert the changes.
How?
I ran
npm installusing node24.11.0and its recommended version, npm11.6.1.Update: Updating only the lockfile did not resolve the issue on Windows. Based on this feedback, I modified thescriptspackage itself to ignore optional packages that are not installed.This change doesn't seem to have been necessary.
Testing Instructions
Let's see if all the CI tests pass.