misc: workaround node esm segfault #12845
Conversation
patrickhulce
force-pushed
the
avoid_esm_segfault
branch
from
cd591a3 to
cd6ee80
Compare
|
moment of truth! let's run GHA a few times. I see it's already passsed once. Passed times: 3 |
| @@ -43,13 +44,14 @@ | |||
| "test-legacy-javascript": "bash lighthouse-core/scripts/test-legacy-javascript.sh", | |||
| "test-docs": "yarn --cwd docs/recipes/auth && yarn jest docs/recipes/integration-test && yarn --cwd docs/recipes/custom-gatherer-puppeteer test", | |||
| "test-proto": "yarn compile-proto && yarn build-proto-roundtrip", | |||
| "unit-core": "yarn jest \"lighthouse-core\"", | |||
| "unit-core": "yarn jest \"lighthouse-core\" --testPathIgnorePatterns=page-functions", | |||
| "unit-core-esm": "yarn jest-esm \"page-functions\"", | |||
There was a problem hiding this comment.
why is page-functions-test.js esm again? I can't remember what required it to be and #12702 is very long to look through :)
There was a problem hiding this comment.
omg!
it was because DOM is esmodules: https://github.com/GoogleChrome/lighthouse/pull/12702/files#diff-d6bbd267b59088c53ed35f4de77f2a9db41287862aa6328e53c24467391baf2bR31
and I saw less crashing if require/import was not mixed, so I switch it all to import for this file.
but a later change ended up removing DOM from this test file #12796
so we could go back to using require as god intended
There was a problem hiding this comment.
oh nice! SGTM. @patrickhulce?
But also haha, the answer to my question was in the very handy comment you left at the top of page-functions-test.js, @connorjclark. I need to actually read things :)
|
We've run this quite a few times, and it looks good. #12847 fixes the current segfault issue. Close for now, @patrickhulce ? We might revive this PR if needed in our esm journey. |
Summary
Attempting to workaround the node segfault in our unit-core tests by splitting out the one esm test.
Related Issues/PRs
ref #12837