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

[3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.12.2.0) #13090

Open
wants to merge 75 commits into
base: release-3.0
Choose a base branch
from

Conversation

nachocodoner
Copy link
Member

@nachocodoner nachocodoner commented Apr 11, 2024

OSS-386
OSS-385


Summary of this PR here

Fix on Windows tests

After the 20.12.2 node.js updates, calling spawn with a .cmd or .bat file requires the shell: true option. This PR adjusts our usage accordingly.

Update

Tests are fixed on Windows as seen on this picture:
image

However, it doesn't finish properly reaching this code, and it times out at the end.

And locally, I can't seem to reproduce the same, locally it finishes properly.

image

Trying to debug the code around spawn of child processes, since maybe one is never stop after last adjusts. But hard to verify it since locally behaves differently.

Copy link

netlify bot commented Apr 11, 2024

Deploy Preview for v3-meteor-api-docs canceled.

Name Link
🔨 Latest commit 977f775
🔍 Latest deploy log https://app.netlify.com/sites/v3-meteor-api-docs/deploys/66462d4dcf38e40008c50fdd

@Grubba27
Copy link
Contributor

you need to build a new dev bundle btw

@nachocodoner nachocodoner changed the title [3.x] Update Typescript 5.4.5 [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 Apr 11, 2024
@nachocodoner nachocodoner changed the title [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.11.1.3) Apr 11, 2024
@nachocodoner nachocodoner changed the title [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.11.1.3) [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.11.1.4) Apr 11, 2024
@nachocodoner nachocodoner changed the title [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.11.1.4) [3.x] Update Node 20.12.2, Npm 10.5.0 and Typescript 5.4.5 (Dev bundle 20.11.1.3) Apr 11, 2024
meteor Outdated Show resolved Hide resolved
@StorytellerCZ
Copy link
Collaborator

Node 20.13.0 is out:
https://github.com/nodejs/node/releases/tag/v20.13.0

@StorytellerCZ
Copy link
Collaborator

The latest release of Node has some reverse on Windows stuff:
https://github.com/nodejs/node/releases/tag/v20.13.1

@nachocodoner
Copy link
Member Author

nachocodoner commented May 15, 2024

Thanks! Once CI responds sucessfully after recent Windows environment failures, we can proceed to 20.13.1. There are still unresolved issues with using the shell: true option for spawn scripts in a Windows environment, this has not been reverted, then self-test Meteor must continue to pass successfully.

The current problem is that CI environment don't consistently give the same success as local environment. We consider migrating to GitHub Actions using a custom runner to have greater control over the environment and see the check green at last.

@nachocodoner
Copy link
Member Author

nachocodoner commented May 16, 2024

Tests pass 🟢!

To summarize what happened in this PR:

  • In the Node 20.12.2 upgrade there is a breaking change on Windows environment requiring to adapt meteor tool on Windows and the spawn of processes with shell: true. Check how another library had to adapt the breaking change similarly.
  • One of our CI checks started to fail, and even after fixing the breaking change, the next error araised was a timedout consistently. Some of the next work then we do is to ensure our CI tests signal green, the same way happened locally in my Windows machine. The phases on this work:
    • AppVeyor check. It triggered timeout, the process was reaching the limits. No plan to upgrade since we transit generally to GitHubActions.
    • GitHub action migration using their runners. One test didn't pass green, a specific test when testing the client part in headless mode. Likely a limitation on Github runners so we prefered to opt for other solution.
    • GitHub action migration using custom runner on a AWS machine. Having more control on the machine setup and replicate what we got locally. After several config attempts, it passed ok at last.

@nachocodoner nachocodoner marked this pull request as ready for review May 16, 2024 03:20
StorytellerCZ
StorytellerCZ previously approved these changes May 16, 2024
denihs
denihs previously approved these changes May 16, 2024
@nachocodoner nachocodoner dismissed stale reviews from denihs and StorytellerCZ via 977f775 May 16, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants