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

Tests: Fix npm tests on macOS #6454

Closed
wants to merge 3 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Apr 26, 2024

The npm tests GHA workflow has recently been failing for the macOS environment on the 6.3 branch:

Not found in manifest. Falling back to download directly from Node
[...]
Unable to find Node version '14' for platform darwin and architecture arm64.

The reason is that GHA's macos-latest test runner has recently been updated to macOS 14 "Sonoma" on Intel Silicone (arm64); OTOH, .nvmrc is pinned to node 14, which isn't available for Apple Silicone (arm64).

If we want to avoid upgrading our entire build chain from node 14 to 16 for a legacy WP branch, then our best course of action is to pin the macOS runner to an Intel-based architecture (amd64).

Per this overview, the best candidate is probably macos-13, as it's apparently the latest Intel-based macOS version runner available to public repositories.

Some prior Slack discussion here: https://wordpress.slack.com/archives/C02RQBWTW/p1714137046024979

Props @swissspidy for helping with this investigation.

Trac ticket: TBD


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@ockham ockham requested a review from swissspidy April 26, 2024 13:40
@ockham ockham self-assigned this Apr 26, 2024
@ockham
Copy link
Contributor Author

ockham commented Apr 26, 2024

Node 14 is now being downloaded successfully. However, the workflow is still failing:

svn: command not found

@ockham
Copy link
Contributor Author

ockham commented Apr 26, 2024

Node 14 is now being downloaded successfully. However, the workflow is still failing:

svn: command not found

This is the reason: https://github.blog/changelog/2024-01-08-subversion-has-been-sunset/

We'll probably need to simply remove the corresponding line, as was done e.g. in https://core.trac.wordpress.org/changeset/57249.

@swissspidy
Copy link
Member

Makes sense to backport that change to a few older branches 👍

cc @desrosj for thoughts

@ockham
Copy link
Contributor Author

ockham commented Apr 26, 2024

Makes sense to backport that change to a few older branches 👍

cc @desrosj for thoughts

I pushed a commit straight to this branch. Think that's okay?

FWIW, our tests layout has changed a bit since 6.3: test-npm.yml is no more; it was absorbed into callable-test-core-build-process.yml and callable-test-gutenberg-build-process.yml.

@swissspidy
Copy link
Member

swissspidy commented Apr 26, 2024

I pushed a commit straight to this branch. Think that's okay?

For testing purposes sure, but for commit history purposes and logistical reasons I'd prefer proper backporting of said commit (including correct svn mergeinfo). Even if that means resolving some merge conflicts / manual adjustments.

Plus the change needs to be done in the 6.4 branch as well.

@desrosj
Copy link
Contributor

desrosj commented Apr 26, 2024

I've been working on #6232 to make all workflows callable the last week or so. It's not quite ready, but this would be a prime example for when this alternative approach would come in handy. If all workflows are callable, we'd only have to update one branch to fix the workflow in all previous ones.

I think that both of these issues could be resolved as part of that effort instead of backporting to all past branches. I do think it's preferable to continue testing with latest in trunk and any branches that use 20.x (6.4 and newer).

If this is blocking things, I don't mind committing this to 6.3. But I would rather fix it properly for all branches at the same time.

@ockham
Copy link
Contributor Author

ockham commented Apr 26, 2024

I've been working on #6232 to make all workflows callable the last week or so. It's not quite ready, but this would be a prime example for when this alternative approach would come in handy. If all workflows are callable, we'd only have to update one branch to fix the workflow in all previous ones.

Makes sense, and sounds like a great improvement!

I think that both of these issues could be resolved as part of that effort instead of backporting to all past branches. I do think it's preferable to continue testing with latest in trunk and any branches that use 20.x (6.4 and newer).

If this is blocking things, I don't mind committing this to 6.3. But I would rather fix it properly for all branches at the same time.

Will this be possible for 6.3 though? After all, the problem there is that it's using Node 14 (which is unavailable on arm64, and thus in macos-latest). (Updating that branch to Node 20 seemed potentially unwieldy to me, as it'd affect our entire JS build chain.)

@ockham
Copy link
Contributor Author

ockham commented Jun 4, 2024

Closing as obsolete per https://core.trac.wordpress.org/changeset/58300 (ede195c).

@ockham ockham closed this Jun 4, 2024
@ockham ockham deleted the fix/npm-tests-on-macos branch June 4, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants