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

ci: add nodejs integration on macos/linux #255

Merged
merged 3 commits into from
May 30, 2024

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented May 9, 2024

Add node-integration test to ensure the vendored packaging will not be removed by accident.

Refs: #250 (comment)
Fixes: #31

@legendecas legendecas changed the title ci: add node-integration ci: add nodejs integration on macos/linux May 9, 2024
strategy:
fail-fast: false
matrix:
os: [macos-latest, macos-14, ubuntu-latest]
Copy link
Member

Choose a reason for hiding this comment

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

macos-latest and macos-14 are exactly the same:
https://github.com/nodejs/gyp-next/actions/runs/9015778952/job/24771097889?pr=255
https://github.com/nodejs/gyp-next/actions/runs/9015778952/job/24771097430?pr=255

See "Set up job" -> "Operating System" and "Runner Image"

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks for pointing this out. I copied this from https://github.com/nodejs/gyp-next/blob/main/.github/workflows/node-gyp.yml#L13. I'll remove this duplication.

Copy link
Contributor

@cclauss cclauss May 9, 2024

Choose a reason for hiding this comment

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

macos-13 is Intel ($runner.arch==X64)
macos-14 is Apple Silicon ($runner.arch==ARM64)
macos-latest is a rolling transition for 12 weeks from April 1st. (Unless someone can point to newer docs from GitHub.)

GitHub Actions blog:

Over the next 12 weeks, jobs using the macos-latest runner label will migrate from macOS 12 (Monterey) to macOS 14 (Sonoma).

https://github.blog/changelog/2024-04-01-macos-14-sonoma-is-generally-available-and-the-latest-macos-runner-image

fail-fast: false
matrix:
os: [macos-latest, macos-14, ubuntu-latest]
python: ["3.8", "3.10", "3.12"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Test the beta? https://www.python.org/download/pre-releases/

Suggested change
python: ["3.8", "3.10", "3.12"]
python: ["3.8", "3.10", "3.12", "3.13"]

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Please add it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Submitted nodejs/node#53190

@cclauss
Copy link
Contributor

cclauss commented May 9, 2024

Why merge all this complexity into a single file?

@legendecas
Copy link
Member Author

Why merge all this complexity into a single file?

Most steps are similar in the integration tests, except from platform differences.

Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Minor optimization

.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
.github/workflows/nodejs.yml Outdated Show resolved Hide resolved
@legendecas legendecas merged commit 42b5e15 into nodejs:main May 30, 2024
46 checks passed
@legendecas legendecas deleted the node-integration branch May 30, 2024 16:59
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.

Test integration with Node.js and node-gyp with GH actions
4 participants