test: fix Windows async-context-frame memory failure#54823
test: fix Windows async-context-frame memory failure#54823nodejs-github-bot merged 1 commit intonodejs:mainfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54823 +/- ##
==========================================
+ Coverage 87.60% 87.61% +0.01%
==========================================
Files 650 650
Lines 182943 182997 +54
Branches 35399 35419 +20
==========================================
+ Hits 160270 160341 +71
+ Misses 15936 15924 -12
+ Partials 6737 6732 -5 |
|
I canceled that run to limit what is running on CI right now. I think there are too many concurrent test runs going causing things to fail out. Trying to get things untangled. Will start it again once things appear more stable. |
|
Ok restarted CI. Some of the other tests may still flake out until this is rebased on tip of |
| // TODO(qard): I think high concurrency causes memory problems on Windows | ||
| // concurrency: tests.length |
There was a problem hiding this comment.
| // TODO(qard): I think high concurrency causes memory problems on Windows | |
| // concurrency: tests.length | |
| // TODO(qard): I think high concurrency causes memory problems on Windows. | |
| // concurrency: tests.length |
|
CI looks good! Let's go with this. |
|
Thanks for managing the CI on that! Sorry for the trouble. 😅 |
|
Can we also revert 407b61c? |
|
Fast-track has been requested by @jasnell. Please 👍 to approve. |
|
Requesting fast-track in order to help improve CI stability. |
Commit Queue failed- Loading data for nodejs/node/pull/54823 ✔ Done loading data for nodejs/node/pull/54823 ----------------------------------- PR info ------------------------------------ Title test: fix Windows async-context-frame memory failure (#54823) Author Stephen Belanger <admin@stephenbelanger.com> (@Qard) Branch Qard:fix-async-context-frame-window-failure -> nodejs:main Labels test, fast-track, author ready, async_local_storage Commits 1 - test: fix Windows async-context-frame memory failure Committers 1 - Stephen Belanger <stephen.belanger@datadoghq.com> PR-URL: https://github.com/nodejs/node/pull/54823 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/54823 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 06 Sep 2024 21:20:38 GMT ✔ Approvals: 4 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/54823#pullrequestreview-2287302578 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/54823#pullrequestreview-2287457244 ✔ - Jake Yuesong Li (@jakecastelli): https://github.com/nodejs/node/pull/54823#pullrequestreview-2287489551 ✔ - Rafael Gonzaga (@RafaelGSS) (TSC): https://github.com/nodejs/node/pull/54823#pullrequestreview-2288434091 ℹ This PR is being fast-tracked ✘ This PR needs to wait 17 more hours to land (or 0 hours if there is 1 more approval (👍) of the fast-track request from collaborators). ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-09-07T00:04:28Z: https://ci.nodejs.org/job/node-test-pull-request/62086/ - Querying data for job/node-test-pull-request/62086/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/10756651585 |
|
Landed in e1e312d |
PR-URL: #54823 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs#54823 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs#54823 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Possible alternative to #54818 to fix the issue with AsyncContextFrame tests crashing with memory errors on Windows. I suspect we're just spinning up too much stuff and eating all the memory.
cc @nodejs/diagnostics