Skip to content

Ci: drop the workflow-wide 8 GB NODE_OPTIONS heap ceiling from validation - #859

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/ci-drop-node-options-heap-override
Open

Ci: drop the workflow-wide 8 GB NODE_OPTIONS heap ceiling from validation#859
AmaadMartin wants to merge 1 commit into
mainfrom
fix/ci-drop-node-options-heap-override

Conversation

@AmaadMartin

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A
  2. Or, if no issue exists, describe the change:
    Problem: validation.yaml sets a workflow-wide NODE_OPTIONS=--max-old-space-size=8192. macos-latest is a 3 CPU / 7 GB runner, so an 8 GB per-process heap ceiling cannot be honoured there. The override also reaches every forked vitest worker.

Solution: I deleted the env: block, so each runner gets the ceiling Node derives from its own physical memory. On Node v22.22.2 that default measures 4144 MB, against 8240 MB under the override. cross-language-integration.yml already runs npm install, npm run build and the cross-language tests on macos-latest with no override.

Worker ceilings do not change. vitest.config.ts sets them through poolOptions.forks.execArgv, which wins over an inherited NODE_OPTIONS (measured: parent 8240 MB, child with execArgv=--max-old-space-size=2048 gets 2096 MB). Right-sizing that value is a separate change (#770).

Collision check: I read the diff of every open fork PR that touches .github/workflows/validation.yaml (#799, #756, #713, #669, #652, #651, #649, #631, #593, #574, #572, #571, #566, #648). None of them adds or removes NODE_OPTIONS, so this does not duplicate or conflict with any of them.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[ ] I have added or updated unit tests for my change. The change deletes three lines of CI config and adds no code to cover, and the repository has no harness that asserts on workflow YAML.
[ ] All unit tests pass locally. The suite runs in this PR's own validation job, on every OS in the matrix, under the default heap ceiling. That run is the test.
Manual End-to-End (E2E) Tests:
Before pushing:

  • python3 -c "import yaml; yaml.safe_load(open('.github/workflows/validation.yaml'))" — parses.
  • git diff --stat1 file changed, 3 deletions(-).
  • grep -rn "NODE_OPTIONS\|max-old-space" .github/ — no output.

One gap: if #756 lands first, this PR's matrix is ubuntu + windows only, and macOS is covered by the post-merge push run.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[ ] I have commented my code, particularly in hard-to-understand areas. A comment explaining a variable that is no longer set would be noise.
[ ] I have added tests that prove my fix is effective or that my feature works. See the note above.
[ ] New and existing unit tests pass locally with my changes. The change touches no code, so the suite is exercised by this PR's validation run rather than locally.

macos-latest is a 3 CPU / 7 GB runner, so --max-old-space-size=8192
cannot be honoured there. Node sizes its own default ceiling from the
host memory, which is correct on every runner in the matrix.

The vitest workers keep their ceiling: vitest.config.ts sets it through
poolOptions.forks.execArgv, which wins over an inherited NODE_OPTIONS.
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.

1 participant